Re: Parent/child context relation in pg_get_backend_memory_contexts()

From: Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Parent/child context relation in pg_get_backend_memory_contexts()
Date: 2024-07-02 13:08:22
Message-ID: CAGPVpCSTR8c+koRzoNnxEvcOgVH2yLDMcnH67z8a5ehKfYiheg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi hackers,

David Rowley <dgrowleyml(at)gmail(dot)com>, 4 Nis 2024 Per, 04:44 tarihinde şunu
yazdı:

> My view on this is that there are a couple of things with the patch
> which could be considered separately:
>
> 1. Should we have a context_id in the view?
> 2. Should we also have an array of all parents?
>

I discussed the above questions with David off-list, and decided to make
some changes in the patch as a result. I'd appreciate any input.

First of all, I agree that previous versions of the patch could make things
seem a bit more complicated than they should be, by having three new
columns (context_id, path, total_bytes_including_children). Especially when
we could already get the same result with several different ways (e.g.
writing a recursive query, using the patch column, and the
total_bytes_including_children column by itself help to know total used
bytes by a contexts and all of its children)

I believe that we really need to have context IDs as it's the only unique
way to identify a context. And I'm for having a parents array as it makes
things easier and demonstrates the parent/child relation explicitly. One
idea to simplify this patch a bit is adding the ID of a context into its
own path and removing the context_id column. As those IDs are temporary, I
don't think they would be useful other than using them to find some kind of
relation by looking into path values of some other rows. So maybe not
having a separate column for IDs but only having the path can help with the
confusion which this patch might introduce. The last element of the patch
would simply be the ID of that particular context.

One nice thing which David pointed out about paths is that level
information can become useful in those arrays. Level can represent the
position of a context in the path arrays of its child contexts. For
example; TopMemoryContext will always be the first element in all paths as
it's the top-most parent, it's also the only context with level 0. So this
relation between levels and indexes in path arrays can be somewhat useful
to link this array with the overall hierarchy of memory contexts.

An example query to get total used bytes including children by using level
info would look like:

WITH contexts AS (
SELECT * FROM pg_backend_memory_contexts
)
SELECT sum(total_bytes)
FROM contexts
WHERE path[( SELECT level+1 FROM contexts WHERE name =
'CacheMemoryContext')] =
(SELECT path[level+1] FROM contexts WHERE name = 'CacheMemoryContext');

Lastly, I created a separate patch to add total_bytes_including_children
columns. I understand that sum of total_bytes of a context and its children
will likely be one of the frequently used cases, not everyone may agree
with having an _including_children column for only total_bytes. I'm open to
hear more opinions on this.

Best Regards,
--
Melih Mutlu
Microsoft

Attachment Content-Type Size
v6-0002-Add-total_bytes_including_children-column.patch application/octet-stream 7.6 KB
v6-0001-Add-path-column-into-pg_backend_memory_contexts.patch application/octet-stream 9.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2024-07-02 13:30:28 Re: Adding skip scan (including MDAM style range skip scan) to nbtree
Previous Message Heikki Linnakangas 2024-07-02 12:59:45 Re: Inline non-SQL SRFs using SupportRequestSimplify