Thread

Re: Enhancing Memory Context Statistics Reporting

Rahila Syed <rahilasyed90@gmail.com> — 2025-07-11T15:31:12Z

Hi,

Please find attached the latest memory context statistics monitoring patch.
It has been redesigned to address several issues highlighted in the thread
[1]
and [2].

Here are some key highlights of the new design:

- All DSA processing has been moved out of the CFI handler function. Now,
all the dynamic shared memory
needed to store the statistics is created and deleted in the client
function.  This change addresses concerns
that DSA APIs are too high level to be safely called from interrupt
handlers. There was also a concern that
DSA API calls might not provide re-entrancy, which could cause issues if
CFI is invoked from a DSA function
in the future.

- The static shared memory array has been replaced with a DSHASH table
which now holds metadata such as
pointers to actual statistics for each process.

- dsm_registry.c APIs are used  for creating and attaching to DSA and
DSHASH table, which helps prevent code
duplication.

-To address the memory leak concern, we create an exclusive memory context
under the NULL context, which
does not fall under the TopMemoryContext tree, to handle all the memory
allocations in ProcessGetMemoryContextInterrupt.
This ensures the memory context created by the function does not affect its
outcome.
The memory context is reset at the end of the function, which helps prevent
any memory leaks.

- Changes made to the mcxt.c file have been relocated to mcxtfuncs.c, which
now contains all the existing
 memory statistics-related functions along with the code for the proposed
function.

The overall flow of a request is as follows:

1. A client backend running the pg_get_process_memory_contexts function
creates a DSA and allocates memory
to store statistics, tracked by DSA pointer. This pointer is stored in a
DSHASH entry for each client querying the
statistics of any process.
The client shares its DSHASH table key with the server process using a
static shared array of keys indexed
by the server's procNumber.  It notifies the server process to publish
statistics by using SendProcSignal.

2. When a PostgreSQL server process handles the request for memory
statistics, the CFI function accesses the
client hash key stored in its procNumber slot of the shared keys array. The
server process then retrieves the
DSHASH entry to obtain the DSA pointer allocated by the client, for storing
the statistics.
 After storing the statistics, it notifies the client through its condition
variable.

3.  Although the DSA is created just once, the memory inside the DSA is
allocated and released by the client
process as soon as it finishes reading the statistics.
If it fails to do so, it is deleted by the before_shmem_exit callback when
the client exits. The client's entry in DSHASH
table is also deleted when the client exits.

4. The DSA and DSHASH table are not created
until  pg_get_process_memory_context function is called.
Once created, any client backend querying statistics and any PostgreSQL
process publishing statistics will
attach to the same area and table.

Please let me know your thoughts.

Thank you,
Rahila Syed

[1]. PostgreSQL: Re: pgsql: Add function to get memory context stats for
processes
<https://www.postgresql.org/message-id/CA%2BTgmoaey-kOP1k5FaUnQFd1fR0majVebWcL8ogfLbG_nt-Ytg%40mail.gmail.com>
[2]. PostgreSQL: Re: Prevent an error on attaching/creating a DSM/DSA from
an interrupt handler.
<https://www.postgresql.org/message-id/flat/8B873D49-E0E5-4F9F-B8D6-CA4836B825CD%40yesql.se#7026d2fe4ab0de6dd5decd32eb9c585a>

On Wed, Apr 30, 2025 at 4:13 PM Daniel Gustafsson <daniel@yesql.se> wrote:

> > On 30 Apr 2025, at 12:14, Peter Eisentraut <peter@eisentraut.org> wrote:
> >
> > On 29.04.25 15:13, Rahila Syed wrote:
> >> Please find attached a patch with some comments and documentation
> changes.
> >> Additionaly, added a missing '\0' termination to "Remaining Totals"
> string.
> >> I think this became necessary after we replaced dsa_allocate0()
> >> with dsa_allocate() is the latest version.
> >
> > >   strncpy(nameptr, "Remaining Totals", namelen);
> > > + nameptr[namelen] = '\0';
> >
> > Looks like a case for strlcpy()?
>
> True.  I did go ahead with the strncpy and nul terminator assignment,
> mostly
> out of muscle memory, but I agree that this would be a good place for a
> strlcpy() instead.
>
> --
> Daniel Gustafsson
>
>