> On 21 Feb 2026, at 00:13, Rahila Syed <rahilasyed90@gmail.com> wrote:
> Please find attached updated and rebased patches which incorporate
> code fixes suggested off-list by Daniel
>
> These changes include a fix to free dsa memory in one of the
> exit paths from the client-side function along with several documentation
> and comment improvements.
Thanks for the new version, I think this is looking pretty good now. The
attached .diff.txt (renamed to keep the CFBot from grabbing it) has mostly
whitespace fixes, but also a few small things which are discussed below:
+ <para>
+ After receiving memory context statistics from the target process, it
+ returns the results as one row per context. If all the contexts don't
Re-reading the docs I think the para's are in the wrong order, as the sentence
above makes little sense to be after the output has already been discussed. I
think this needs to go earlier, see the attached .diff.txt for what I am
proposing.
+ /*
+ * The client process should have created the required DSA and DSHash
+ * table. Here we just attach to those.
+ */
+ if (MemoryStatsDsaArea == NULL)
+ MemoryStatsDsaArea = GetNamedDSA("memory_context_statistics_dsa",
+ &found);
+
+ if (MemoryStatsDsHash == NULL)
+ MemoryStatsDsHash = GetNamedDSHash("memory_context_statistics_dshash",
+ &memctx_dsh_params, &found);
I think this needs an expanded comment discussing why there is no need the
check the DSA and DShash after the GetNamed_ functions, since they can be NULL
going in to this. The attached .diff.txt has a proposal along with an
Assertion to keep future changes of the API from risking subtly breaking this
assumption.
+REGRESS = test_memcontext_reporting
Since the test module doesn't contain any pg_regress style .sql/.out tests so
this line in the Makefile cause a test failure when running with Autoconf.
+$node->append_conf(
+ 'postgresql.conf',
+ qq[
+max_connections = 100
+log_statement = none
+restart_after_crash = false
+]);
Why do we need to set max_connections for this test?
+#Server should have thrown error
+$node->psql(
+ 'postgres',
+ qq(select pg_get_process_memory_contexts($pid, true);),
+ stderr => \$psql_err);
This test doesn't validate that the server actually errored does it? (There is
no proposed fix in the attached.)
--
Daniel Gustafsson
