Re: Parent/child context relation in pg_get_backend_memory_contexts()

On 2024-01-16 18:41, Melih Mutlu wrote:
> Hi,
>
> Thanks for reviewing.
>
> torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, 10 Oca 2024 Çar, 09:37
> tarihinde şunu yazdı:
>
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>context_id</structfield> <type>int4</type>
>>> + </para>
>>> + <para>
>>> + Current context id. Note that the context id is a
>> temporary id
>>> and may
>>> + change in each invocation
>>> + </para></entry>
>>> + </row>
>>> +
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>path</structfield> <type>int4[]</type>
>>> + </para>
>>> + <para>
>>> + Path to reach the current context from TopMemoryContext.
>>> Context ids in
>>> + this list represents all parents of the current context.
>> This
>>> can be
>>> + used to build the parent and child relation
>>> + </para></entry>
>>> + </row>
>>> +
>>> + <row>
>>> + <entry role="catalog_table_entry"><para
>>> role="column_definition">
>>> + <structfield>total_bytes_including_children</structfield>
>>> <type>int8</type>
>>> + </para>
>>> + <para>
>>> + Total bytes allocated for this memory context including
>> its
>>> children
>>> + </para></entry>
>>> + </row>
>>
>> These columns are currently added to the bottom of the table, but it
>> may
>> be better to put semantically similar items close together and
>> change
>> the insertion position with reference to other system views. For
>> example,
>>
>> - In pg_group and pg_user, 'id' is placed on the line following
>> 'name',
>> so 'context_id' be placed on the line following 'name'
>> - 'path' is similar with 'parent' and 'level' in that these are
>> information about the location of the context, 'path' be placed to
>> next
>> to them.
>>
>> If we do this, orders of columns in the system view should be the
>> same,
>> I think.
>
> I've done what you suggested. Also moved
> "total_bytes_including_children" right after "total_bytes".
>
>> 14dd0f27d have introduced new macro foreach_int.
>> It seems to be able to make the code a bit simpler and the commit
>> log
>> says this macro is primarily intended for use in new code. For
>> example:
>
> Makes sense. Done.

Thanks for updating the patch!

> + Current context id. Note that the context id is a temporary id
> and may
> + change in each invocation
> + </para></entry>
> + </row>

It clearly states that the context id is temporary, but I am a little
concerned about users who write queries that refer to this view multiple
times without using CTE.

If you agree, how about adding some description like below you mentioned
before?

> We still need to use cte since ids are not persisted and might change
> in
> each run of pg_backend_memory_contexts. Materializing the result can
> prevent any inconsistencies due to id change. Also it can be even good
> for
> performance reasons as well.

We already have additional description below the table which explains
each column of the system view. For example pg_locks:
https://www.postgresql.org/docs/devel/view-pg-locks.html

Also giving an example query something like this might be useful.

-- show all the parent context names of ExecutorState
with contexts as (
select * from pg_backend_memory_contexts
)
select name from contexts where array[context_id] <@ (select path from
contexts where name = 'ExecutorState');

--
Regards,

--
Atsushi Torikoshi
NTT DATA Group Corporation