public inbox for [email protected]
help / color / mirror / Atom feedFrom: torikoshia <[email protected]>
To: Rahila Syed <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Subject: Re: Enhancing Memory Context Statistics Reporting
Date: Tue, 12 Aug 2025 09:34:00 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH2L28t=O1k+5wdcP88rgty3OLZisTU72WGH8Dp2MxJjwn7=fw@mail.gmail.com>
References: <CAExHW5skNvp265od6XPs0O-RL3cwtgHW3N87Ob0+nByZ=_HzAA@mail.gmail.com>
<[email protected]>
<CAH2L28v9EU4dxKUpvMt_CFAzG72CYusPWxADsgT=cwFJP-fP0A@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAH2L28s+s4JJdPz2RkALzOvXpxXUcmm=fvg-Y6M82g9Cp=bB-w@mail.gmail.com>
<[email protected]>
<5bxhxniyvjyfldi7yjxcnxkl3i2ghci2grjyeclrbkfqnyhowk@dfkqzbvtbpml>
<[email protected]>
<tesneyk3z2dtrjgwlmkw2wbr7e3olwkowlpke6kl463hfhxedb@fyyqsnwjcp4l>
<CAH2L28tp8RMa0CrCgdCJw20vFzeGQMuHkXAoPgYC5JZuXY8_+g@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAH2L28vt16C9xTuK+K7QZvtA3kCNWXOEiT=gEekUw3Xxp9LVQw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAH2L28sc-rEhyntPLoaC2XUa0ZjS5ka6KzEbuSVxQBBnUYu1KQ@mail.gmail.com>
<CAH2L28vCCgye_+kJt22RAFzZfYbO7ytSrp-hR6-SenBcm_cN+w@mail.gmail.com>
<CAH2L28t=O1k+5wdcP88rgty3OLZisTU72WGH8Dp2MxJjwn7=fw@mail.gmail.com>
On 2025-08-08 18:26, Rahila Syed wrote:
Hi, thanks for working on this again.
> Hi,
>
> CFbot indicated that the patch requires a rebase, so I've attached an
> updated version.
Here are some comments and questions for v32 patch:
> --- a/doc/src/sgml/func/func-admin.sgml
> +++ b/doc/src/sgml/func/func-admin.sgml
> @@ -251,6 +251,137 @@
> <literal>false</literal> is returned.
> </para></entry>
> </row>
> +
> + <row>
> + <entry role="func_table_entry"><para role="func_signature">
> + <indexterm>
> + <primary>pg_get_process_memory_contexts</primary>
This function is added at the end of Table "9.96. Server Signaling
Functions", but since pg_get_process_memory_contexts outputs essentially
the same information as pg_log_backend_memory_contexts, it might be
better to place them next to each other in the table.
> + <parameter>stats_timestamp</parameter> <type>timestamptz</type> )
> +typedef struct MemoryStatsDSHashEntry
> +{
> + char key[64];
> + ConditionVariable memcxt_cv;
> + int proc_id;
> + int total_stats;
> + bool summary;
> + dsa_pointer memstats_dsa_pointer;
> + TimestampTz stats_timestamp;
> +} MemoryStatsDSHashEntry;
stats_timestamp appears only in the two places below in the patch, but
it does not seem to be actually output.
Is this column unnecessary?
=# select * from pg_get_process_memory_contexts(pg_backend_pid(),
true, 10);
-[ RECORD 1 ]----+-----------------------------
name | TopMemoryContext
ident | [NULL]
type | AllocSet
path | {1}
level | 1
total_bytes | 222400
total_nblocks | 8
free_bytes | 4776
free_chunks | 8
used_bytes | 217624
num_agg_contexts | 1
Specifying 0 for timeout causes a crash:
=# select * from pg_get_process_memory_contexts(74526, true, 0);
(0 rows)
=# select 1;
WARNING: terminating connection because of crash of another server
process
DETAIL: The postmaster has commanded this server process to roll back
the current transaction and exit, because another server process exited
abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
Should 0 be handled safely and treated as “no timeout”, or rejected as
an error?
Similarly, specifying a negative value for timeout still works:
=# select * from pg_get_process_memory_contexts(30590, true, -10);
It might be better to reject negative values similar to
pg_terminate_backend().
> context_id_lookup =
> hash_create("pg_get_remote_backend_memory_contexts",
> + /* Retreive the client key fo publishing statistics */
fo -> for?
> + * If the publishing backend does not respond before the condition
> variable
> + * times out, which is set to MEMSTATS_WAIT_TIMEOUT, retry given that
> there is
> + * time left within the timeout specified by the user, before giving
> up and
> + * returning previously published statistics, if any. If no previous
> statistics
> + * exist, return NULL.
> + */
> +#define MEMSTATS_WAIT_TIMEOUT 100
MEMSTATS_WAIT_TIMEOUT is defined, but it doesn’t seem to be used.
--
Regards,
--
Atsushi Torikoshi
Seconded from NTT DATA Japan Corporation to SRA OSS K.K.
view thread (52+ 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]
Subject: Re: Enhancing Memory Context Statistics Reporting
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