public inbox for [email protected]  
help / color / mirror / Atom feed
From: torikoshia <[email protected]>
To: Rahila Syed <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Enhancing Memory Context Statistics Reporting
Date: Mon, 08 Dec 2025 10:26:33 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH2L28sMK4qyo9xH9Jy7UTj1seZDDi4+ATjSSKTcrfcvk_V=ig@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>
	<[email protected]>
	<CAH2L28sF02nZj5vEL49EsTx+TfYpB3dFgShgn0=W1V-uFZjejw@mail.gmail.com>
	<[email protected]>
	<CAH2L28uL=Vm9E4Uf=tkg2Ki7_uDuAXORHVyCA7K_L=PAQ2p4sA@mail.gmail.com>
	<[email protected]>
	<CAH2L28tACFygy9oXbTA8pPQ6KE8uDiO9CgAhku7ERNpdj1YRTw@mail.gmail.com>
	<[email protected]>
	<CAH2L28t=c_iBj0+LoKX0TSTrNJB2nTgH6MPD7QgRb8kSBZ7HVg@mail.gmail.com>
	<CAH2L28v5qXPpfmVLFDcMFL4MMqsKP92FXhW2eosHA5LbatKnrw@mail.gmail.com>
	<[email protected]>
	<CAH2L28ucG7zLjLRU4m_hCaGfo_UwUyw89OZR9Q8t8YMriyijgw@mail.gmail.com>
	<CAH2L28sMK4qyo9xH9Jy7UTj1seZDDi4+ATjSSKTcrfcvk_V=ig@mail.gmail.com>

On 2025-11-28 18:22, Rahila Syed wrote:

Hi,

> I'm attaching the updated patches, which primarily include cleanup and
> have been rebased
> following the CFbot report.

Thanks for updating the patch!

I observed an assertion failure when forcing a timeout as follows:

```
$ psql
   (pid:38587)=#

$ kill -s SIGSTOP 38587

$ psql
   (pid:38618) =# select * from pg_get_process_memory_contexts(38587, 
false);
     name  | ident  |  type  | level  |  path  | total_bytes | 
total_nblocks | free_bytes | free_chunks | used_bytes | num_agg_contexts
     
--------+--------+--------+--------+--------+-------------+---------------+------------+-------------+------------+------------------
      [NULL] | [NULL] | [NULL] | [NULL] | [NULL] |      [NULL] |        
[NULL] |     [NULL] |      [NULL] |     [NULL] |           [NULL]
     (1 row)

    Time: 5013.515 ms (00:05.014)

$ kill -s SIGCONT 38587

$ tail postgresql.log

   TRAP: failed Assert("client_keys[MyProcNumber] != -1"), File: 
"mcxtfuncs.c", Line: 881, PID: 38587
   0   postgres                            0x0000000104943400 
ExceptionalCondition + 216
   1   postgres                            0x000000010480f738 
ProcessGetMemoryContextInterrupt + 140
   2   postgres                            0x00000001046f2710 
ProcessInterrupts + 3008
   3   postgres                            0x00000001046f1a78 
ProcessClientReadInterrupt + 80
   4   postgres                            0x0000000104433994 secure_read 
+ 404
   5   postgres                            0x00000001044411dc pq_recvbuf 
+ 260
   6   postgres                            0x0000000104441088 pq_getbyte 
+ 96
   7   postgres                            0x00000001046fa0fc 
SocketBackend + 44
   8   postgres                            0x00000001046f6d3c ReadCommand 
+ 44
   9   postgres                            0x00000001046f6284 
PostgresMain + 2900
   10  postgres                            0x00000001046ed558 
BackendInitialize + 0
   11  postgres                            0x00000001045c0a48 
postmaster_child_launch + 456
   12  postgres                            0x00000001045c8520 
BackendStartup + 304
   13  postgres                            0x00000001045c636c ServerLoop 
+ 372
   14  postgres                            0x00000001045c4e24 
PostmasterMain + 6448
   15  postgres                            0x0000000104445b4c main + 924
   16  dyld                                0x0000000188662b98 start + 
6076
   2025-12-08 07:35:32.608 JST   [38540] LOG:  00000: client backend (PID 
38587) was terminated by signal 6: Abort trap: 6
   2025-12-08 07:35:32.608 JST   [38540] LOCATION:  LogChildExit, 
postmaster.c:2872
```


Below are comments regarding the v42-0001 patch:

> In order to not block on busy processes, we have hardcoded
> the number of seconds during which to retry before timing out.
> In the case where no statistics are published within the set
> timeout, NULL is returned.

It might be good to also document in func-admin.sgml that the function 
times out after 5 seconds when the target backend does not respond, and 
that in such a case NULLs are returned.

+    * If DSA exists, created by another process requesting statistics, 
attach
+    * to it. We expect the client process to create required DSA and 
Dshash
+    * table.
+    */
+   if (MemoryStatsDsaArea == NULL)
+       MemoryStatsDsaArea = 
GetNamedDSA("memory_context_statistics_dsa",
+                                        &found);
+
+   if (MemoryStatsDsHash == NULL)
+       MemoryStatsDsHash = 
GetNamedDSHash("memory_context_statistics_dshash",
+                                          &memctx_dsh_params, &found);

 From the comment, it sounded to me as if the client executing 
pg_get_process_memory_contexts() might not create the DSA in some cases.
Is it correct to assume that such a situation can happen?
In [1], as a response to concerns about using DSA inside a CFI handler, 
you wrote that “all the dynamic shared memory needed to store the 
statistics is created and deleted in the client function”.
So I understood that it would never create the DSA inside the CFI 
handler.
If that understanding is correct, perhaps the comment should be reworded 
to make that clear.

+   context_id_lookup = 
hash_create("pg_get_remote_backend_memory_contexts",

This appears to use the old function name. Should this be updated to 
"pg_get_process_memory_contexts" instead?

[1] 
https://www.postgresql.org/message-id/CAH2L28sc-rEhyntPLoaC2XUa0ZjS5ka6KzEbuSVxQBBnUYu1KQ%40mail.gma...


-- 
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