MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="=_c994a2a1fc55a1708b7f5ffea845b190"
Date: Mon, 24 Aug 2020 13:01:42 +0900
From: torikoshia <torikoshia@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>, Kasahara Tatsuhito
 <kasahara.tatsuhito@gmail.com>, Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>, Robert Haas
 <robertmhaas@gmail.com>
Subject: Re: Creating a function for exposing memory usage of backend process
In-Reply-To: <20200822121641.GE2108@paquier.xyz>
References: <261eebda3e1909475162b968e5a4ad2b@oss.nttdata.com>
 <66ae5b88-208d-96fa-c20a-fb539b597859@oss.nttdata.com>
 <20200819135545.GC19121@paquier.xyz> <1414992.1597849297@sss.pgh.pa.us>
 <20200819225343.c7rxteydi35nhrdp@alap3.anarazel.de>
 <1554676.1597886946@sss.pgh.pa.us>
 <20200820014301.5pr7qjugnvpovu62@alap3.anarazel.de>
 <d97ad430-444d-9ec8-21dd-a1584569309b@oss.nttdata.com>
 <CAP0=ZVJ5VZsUYLFkc-E5_JHTc1ZdemX28VgQqUXJLTywC=9hfA@mail.gmail.com>
 <a8f3c385e9edeba4833c712270d6e6d9@oss.nttdata.com>
 <20200822121641.GE2108@paquier.xyz>
Message-ID: <c9515ae497b4a077360a31547782acb8@oss.nttdata.com>
User-Agent: Roundcube Webmail/1.1.9
Precedence: bulk

--=_c994a2a1fc55a1708b7f5ffea845b190
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII;
 format=flowed

On 2020-08-22 21:18, Michael Paquier wrote:

Thanks for reviewing!

> On Fri, Aug 21, 2020 at 11:27:06PM +0900, torikoshia wrote:
>> OK. Added a regression test on sysviews.sql.
>> (0001-Added-a-regression-test-for-pg_backend_memory_contex.patch)
>> 
>> Fujii-san gave us an example, but I added more simple one considering
>> the simplicity of other tests on that.
> 
> What you have sent in 0001 looks fine to me.  A small test is much
> better than nothing.
> 
>> Added a patch for relocating the codes to mcxtfuncs.c.
>> (patches/0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch)
> 
> The same code is moved around line-by-line.
> 
>> Of course, this restriction makes pg_backend_memory_contexts hard to 
>> use
>> when the user of the target session is not granted pg_monitor because 
>> the
>> scope of this view is session local.
>> 
>> In this case, I imagine additional operations something like 
>> temporarily
>> granting pg_monitor to that user.
> 
> Hmm.  I am not completely sure either that pg_monitor is the best fit
> here, because this view provides information about a bunch of internal
> structures.  Something that could easily be done though is to revoke
> the access from public, and then users could just set up GRANT
> permissions post-initdb, with pg_monitor as one possible choice.  This
> is the safest path by default, and this stuff is of a caliber similar
> to pg_shmem_allocations in terms of internal contents.

I think this is a better way than what I did in
0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch.

Attached a patch.

> 
> It seems to me that you are missing one "REVOKE ALL on
> pg_backend_memory_contexts FROM PUBLIC" in patch 0003.
> 
> By the way, if that was just for me, I would remove used_bytes, which
> is just a computation from the total and free numbers.  I'll defer
> that point to Fujii-san.
> --
> Michael


On 2020/08/20 2:59, Kasahara Tatsuhito wrote:
>>> I totally agree that it's not *enough*, but in contrast to you I 
>>> think
>>> it's a good step. Subsequently we should add a way to get any 
>>> backends
>>> memory usage.
>>> It's not too hard to imagine how to serialize it in a way that can be
>>> easily deserialized by another backend. I am imagining something like
>>> sending a procsignal that triggers (probably at CFR() time) a backend 
>>> to
>>> write its own memory usage into pg_memusage/<pid> or something 
>>> roughly
>>> like that.
>> 
>> Sounds good. Maybe we can also provide the SQL-callable function
>> or view to read pg_memusage/<pid>, to make the analysis easier.
> +1


I'm thinking about starting a new thread to discuss exposing other
backend's memory context.


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION


--=_c994a2a1fc55a1708b7f5ffea845b190
Content-Transfer-Encoding: base64
Content-Type: text/x-diff;
 name=0001-Previously-pg_backend_memory_contexts-doesn-t-have-a.patch
Content-Disposition: attachment;
 filename=0001-Previously-pg_backend_memory_contexts-doesn-t-have-a.patch;
 size=1704

RnJvbSBkYzRmYWRlOTExMWRjM2Y5MWU5OTJjNGQ1YWYzOTNkZDVlZDAzMjcwIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBdHN1c2hpIFRvcmlrb3NoaSA8dG9yaWtvc2hpYUBvc3MubnR0
ZGF0YS5jb20+CkRhdGU6IE1vbiwgMjQgSnVsIDIwMjAgMTE6MTQ6MzIgKzA5MDAKU3ViamVjdDog
W1BBVENIXSBQcmV2aW91c2x5IHBnX2JhY2tlbmRfbWVtb3J5X2NvbnRleHRzIGRvZXNuJ3QgaGF2
ZSBhbnkKIHJlc3RyaWN0aW9uIGFuZCBhbnlvbmUgY291bGQgYWNjZXNzIGl0LiBIb3dldmVyLCB0
aGlzIHZpZXcgY29udGFpbnMgc29tZQogaW50ZXJuYWwgaW5mb3JtYXRpb24gb2YgdGhlIG1lbW9y
eSBjb250ZXh0LiBUaGlzIHBvbGljeSBjb3VsZCBjYXVzZSBzZWN1cml0eQogaXNzdWVzLiBUaGlz
IHBhdGNoIHJldm9rZXMgYWxsIG9uIHBnX3NobWVtX2FsbG9jYXRpb25zIGZyb20gcHVibGljIGFu
ZCBvbmx5CiB0aGUgc3VwZXJ1c2VycyBjYW4gYWNjZXNzIGl0LgoKLS0tCiBkb2Mvc3JjL3NnbWwv
Y2F0YWxvZ3Muc2dtbCAgICAgICAgICAgfCA0ICsrKysKIHNyYy9iYWNrZW5kL2NhdGFsb2cvc3lz
dGVtX3ZpZXdzLnNxbCB8IDMgKysrCiAyIGZpbGVzIGNoYW5nZWQsIDcgaW5zZXJ0aW9ucygrKQoK
ZGlmZiAtLWdpdCBhL2RvYy9zcmMvc2dtbC9jYXRhbG9ncy5zZ21sIGIvZG9jL3NyYy9zZ21sL2Nh
dGFsb2dzLnNnbWwKaW5kZXggMTIzMmIyNGU3NC4uOWZlMjYwZWNmZiAxMDA2NDQKLS0tIGEvZG9j
L3NyYy9zZ21sL2NhdGFsb2dzLnNnbWwKKysrIGIvZG9jL3NyYy9zZ21sL2NhdGFsb2dzLnNnbWwK
QEAgLTk2OTcsNiArOTY5NywxMCBAQCBTQ1JBTS1TSEEtMjU2JDxyZXBsYWNlYWJsZT4mbHQ7aXRl
cmF0aW9uIGNvdW50Jmd0OzwvcmVwbGFjZWFibGU+OjxyZXBsYWNlYWJsZT4mbAogICAgPC90Z3Jv
dXA+CiAgIDwvdGFibGU+CiAKKyAgPHBhcmE+CisgICBCeSBkZWZhdWx0LCB0aGUgPHN0cnVjdG5h
bWU+cGdfYmFja2VuZF9tZW1vcnlfY29udGV4dHM8L3N0cnVjdG5hbWU+IHZpZXcgY2FuIGJlCisg
ICByZWFkIG9ubHkgYnkgc3VwZXJ1c2Vycy4KKyAgPC9wYXJhPgogIDwvc2VjdDE+CiAKICA8c2Vj
dDEgaWQ9InZpZXctcGctY29uZmlnIj4KZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2NhdGFsb2cv
c3lzdGVtX3ZpZXdzLnNxbCBiL3NyYy9iYWNrZW5kL2NhdGFsb2cvc3lzdGVtX3ZpZXdzLnNxbApp
bmRleCBiYTVhMjNhYzI1Li5hMmQ2MTMwMmY5IDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9jYXRh
bG9nL3N5c3RlbV92aWV3cy5zcWwKKysrIGIvc3JjL2JhY2tlbmQvY2F0YWxvZy9zeXN0ZW1fdmll
d3Muc3FsCkBAIC01NTcsNiArNTU3LDkgQEAgUkVWT0tFIEVYRUNVVEUgT04gRlVOQ1RJT04gcGdf
Z2V0X3NobWVtX2FsbG9jYXRpb25zKCkgRlJPTSBQVUJMSUM7CiBDUkVBVEUgVklFVyBwZ19iYWNr
ZW5kX21lbW9yeV9jb250ZXh0cyBBUwogICAgIFNFTEVDVCAqIEZST00gcGdfZ2V0X2JhY2tlbmRf
bWVtb3J5X2NvbnRleHRzKCk7CiAKK1JFVk9LRSBBTEwgT04gcGdfYmFja2VuZF9tZW1vcnlfY29u
dGV4dHMgRlJPTSBQVUJMSUM7CitSRVZPS0UgRVhFQ1VURSBPTiBGVU5DVElPTiBwZ19nZXRfYmFj
a2VuZF9tZW1vcnlfY29udGV4dHMoKSBGUk9NIFBVQkxJQzsKKwogLS0gU3RhdGlzdGljcyB2aWV3
cwogCiBDUkVBVEUgVklFVyBwZ19zdGF0X2FsbF90YWJsZXMgQVMKLS0gCjIuMTguMQoK
--=_c994a2a1fc55a1708b7f5ffea845b190--