Re: Creating a function for exposing memory usage of backend process

Thanks for all your comments!

Thankfully it seems that this feature is regarded as not
meaningless one, I'm going to do some improvements.


On Wed, Aug 19, 2020 at 10:56 PM Michael Paquier <michael@paquier.xyz> 
wrote:
> On Wed, Aug 19, 2020 at 06:12:02PM +0900, Fujii Masao wrote:
>> On 2020/08/19 17:40, torikoshia wrote:
>>> Yes, I didn't add regression tests because of the unstability of the 
>>> output.
>>> I thought it would be OK since other views like pg_stat_slru and 
>>> pg_shmem_allocations
>>> didn't have tests for their outputs.
>> 
>> You're right.
> 
> If you can make a test with something minimal and with a stable
> output, adding a test is helpful IMO, or how can you make easily sure
> that this does not get broken, particularly in the event of future
> refactorings, or even with platform-dependent behaviors?

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.


On Thu, Aug 20, 2020 at 12:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> > By the way, I was looking at the code that has been committed, and I
> > think that it is awkward to have a SQL function in mcxt.c, which is a
> > rather low-level interface.  I think that this new code should be
> > moved to its own file, one suggestion for a location I have being
> > src/backend/utils/adt/mcxtfuncs.c.
> 
> I agree with that,

Thanks for pointing out.
Added a patch for relocating the codes to mcxtfuncs.c.
(patches/0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch)


> On Thu, Aug 20, 2020 at 11:09 AM Fujii Masao 
> <masao.fujii@oss.nttdata.com> wrote:
> On 2020/08/20 0:01, Tom Lane wrote:
>> Given the lack of clear use-case, and the possibility (admittedly
>> not strong) that this is still somehow a security hazard, I think
>> we should revert it.  If it stays, I'd like to see restrictions
>> on who can read the view.

> For example, allowing only the role with pg_monitor to see this view?

Attached a patch adding that restriction.
(0001-Restrict-the-access-to-pg_backend_memory_contexts-to.patch)

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.

Thoughts?


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION