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.


Reply via email to