public inbox for [email protected]  

help / color / mirror / Atom feed

From: torikoshia <[email protected]>
To: Melih Mutlu <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Parent/child context relation in pg_get_backend_memory_contexts()
Date: Fri, 19 Jan 2024 17:41:45 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAGPVpCSvNDXy1vDTjgGVCOCAndQ6iB_LVyjb1GNATEqdx8zLHw@mail.gmail.com>
References: <CAGPVpCThLyOsj3e_gYEvLoHkr5w=tadDiN_=z2OwsK3VJppeBA@mail.gmail.com>
	<CAGPVpCRNGd4V6MsxOttDZG4Hbf-4c1rrQKL4CsLyhaSY4sNkjA@mail.gmail.com>
	<[email protected]>
	<CAGPVpCQjpcU=T9wtzWX+dU5LkXnPc2OBqNHoZmKHWFFH8YL7ZQ@mail.gmail.com>
	<[email protected]>
	<CAGPVpCT0xuoBvx0=myzKYmv02bQ64BC96Z_-dVKckH3++Jg+DA@mail.gmail.com>
	<[email protected]>
	<CAGPVpCSvNDXy1vDTjgGVCOCAndQ6iB_LVyjb1GNATEqdx8zLHw@mail.gmail.com>

On 2024-01-16 18:41, Melih Mutlu wrote:
> Hi,
> 
> Thanks for reviewing.
> 
> torikoshia <[email protected]>, 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

view thread (20+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Parent/child context relation in pg_get_backend_memory_contexts()
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox