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

On 2020/07/03 11:45, torikoshia wrote:
> On Wed, Jul 1, 2020 at 10:15 PM torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> wrote:
>> I'm going to do some renaming and transportations.
>>
>> - view name: pg_memory_contexts

I like more specific name like pg_backend_memory_contexts.
But I'd like to hear more opinions about the name from others.

>> - function name: pg_get_memory_contexts()
>> - source file: mainly src/backend/utils/mmgr/mcxt.c
>
> Attached an updated patch.

Thanks for updating the patch!

+ <structfield>level</structfield> <type>integer</type>

In catalog.sgml, "int4" and "int8" are used in other catalogs tables.
So "integer" in the above should be "int4"?

+ <structfield>total_bytes</structfield> <type>bigint</type>

"bigint" should be "int8"?

+ Identification information of the memory context. This field is truncated if the identification field is longer than 1024 characters

"characters" should be "bytes"?

It's a bit confusing to have both "This field" and "the identification field"
in one description. What about just "This field is truncated at 1024 bytes"?

+ <para>
+ Total bytes requested from malloc

Isn't it better not to use "malloc" in the description? For example,
what about something like "Total bytes allocated for this memory context"?

+#define PG_STAT_GET_MEMORY_CONTEXT_COLS 9

Isn't it better to rename this to PG_GET_MEMORY_CONTEXTS_COLS
for the consistency with the function name?

+ memset(nulls, 0, sizeof(nulls));

"values[]" also should be initialized with zero?

Regards,

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