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

On 2020/08/24 21:56, torikoshia wrote:
> On 2020-08-24 13:13, Fujii Masao wrote:
>> On 2020/08/24 13:01, torikoshia wrote:
>>> 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.
>>
>> +1
>>
>> But as I proposed upthread, what about a bit complicated test as follows,
>> e.g., to confirm that the internal logic for level works expectedly?
>>
>>      SELECT name, ident, parent, level, total_bytes >= free_bytes FROM
>> pg_backend_memory_contexts WHERE level = 0;
>
> OK!
> Attached an updated patch.

Thanks for updating the patch! Looks good to me.
Barring any objection, I will commit this patch.

>
>>
>>
>>>>
>>>>> Added a patch for relocating the codes to mcxtfuncs.c.
>>>>> (patches/0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch)
>>
>> Thanks for the patch! Looks good to me.
>> Barring any objection, I will commit this patch at first.
>>
>>
>>>>
>>>> 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.
>>
>> You mean 0001-Restrict-the-access-to-pg_backend_memory_contexts-to.patch?
>
> Oops, I meant 0001-Restrict-the-access-to-pg_backend_memory_contexts-to.patch.

This patch also looks good to me. Thanks!

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION