Subject: Re: Enhancing Memory Context Statistics Reporting

From: Fujii Masao <[email-masked]>
Date: Wed, 9 Apr 2025 01:41:49 +0900

On 2025/04/08 18:46, Daniel Gustafsson wrote:
>> On 8 Apr 2025, at 10:03, Daniel Gustafsson <[email-masked]> wrote:
> 
>> There was a bug in the shmem init function which caused it to fail on Windows,
>> the attached fixes that.
> 
> With this building green in CI over several re-builds, and another pass over
> the docs and code with pgindent etc done, I pushed this earlier today.  A few
> BF animals have built green so far but I will continue to monitor it.

Thanks for committing this feature!

I noticed that the third argument of pg_get_process_memory_contexts() is named
"retries" in pg_proc.dat, while the documentation refers to it as "timeout".
Since "retries" is misleading, how about renaming it to "timeout" in pg_proc.dat?
Patch attached.

Also, as I mentioned earlier, I encountered an issue when calling
pg_get_process_memory_contexts() on the PID of a backend that had just
encountered an error but hadn't finished rolling back. It led to
the following situation:

    Session 1 (PID=70011):
    =# begin;
    =# select 1/0;
    ERROR:  division by zero

    Session 2:
    =# select * from pg_get_process_memory_contexts(70011, false, 10);

    Session 1 terminated with:
    ERROR:  ResourceOwnerEnlarge called after release started
    FATAL:  terminating connection because protocol synchronization was lost

Shouldn't this be addressed?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From a79084fe3caaf791f2aa8d466603c085ccb8c5af Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
Date: Wed, 9 Apr 2025 01:27:48 +0900
Subject: [PATCH v1] Rename misleading argument in
 pg_get_process_memory_contexts().

Previously, the third argument of pg_get_process_memory_contexts()
was named retries in pg_proc.dat, even though it actually specifies
a timeout value in seconds. This name was misleading to users and
inconsistent with the documentation, which correctly referred to it
as timeout.

This commit renames the argument to timeout in pg_proc.dat to
improve clarity and maintain consistency with the documentation.
---
 src/include/catalog/pg_proc.dat | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4708f55be18..62beb71da28 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8578,7 +8578,7 @@
   prorettype => 'record', proargtypes => 'int4 bool float8',
   proallargtypes => '{int4,bool,float8,text,text,text,_int4,int4,int8,int8,int8,int8,int8,int4,timestamptz}',
   proargmodes => '{i,i,i,o,o,o,o,o,o,o,o,o,o,o,o}',
-  proargnames => '{pid, summary, retries, name, ident, type, path, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, num_agg_contexts, stats_timestamp}',
+  proargnames => '{pid, summary, timeout, name, ident, type, path, level, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes, num_agg_contexts, stats_timestamp}',
   prosrc => 'pg_get_process_memory_contexts' },
 
 # non-persistent series generator
-- 
2.49.0

---

From: Daniel Gustafsson <[email-masked]>
Date: Tue, 8 Apr 2025 18:44:41 +0200

> On 8 Apr 2025, at 18:41, Fujii Masao <[email-masked]> wrote:

Ugh, that's my bad.  It was changed from using retries to a timeout and I
missed that.

> Shouldn't this be addressed?

Sorry, this must've been missed in this fairly lon thread, will have a look at
it tonight.

--
Daniel Gustafsson

---

From: Daniel Gustafsson <[email-masked]>
Date: Tue, 8 Apr 2025 23:27:41 +0200

> "retries" in pg_proc.dat, while the documentation refers to it as "timeout".

I've committed this patch as it was obviously correct, thanks!

> encountered an error but hadn't finished rolling back. It led to

I reconfirmed that the bugfix that Rahila shared in [0] fixes this issue (and
will fix others like it, as it's not related to this patch in particular but is
a bug in DSM attaching).  My plan is to take that for a more thorough review
and test tomorrow and see how far it can be safely backpatched.  Thanks for
bringing this up, sorry about it getting a bit lost among all the emails.

--
Daniel Gustafsson

[0] [email-masked]

---

From: Fujii Masao <[email-masked]>
Date: Wed, 9 Apr 2025 08:28:09 +0900

> I've committed this patch as it was obviously correct, thanks!

Thanks a lot!

Since pg_proc.dat was modified, do we need to bump the catalog version?

> bringing this up, sorry about it getting a bit lost among all the emails.

Appreciate your work on this!

Regards,