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 <dan...@yesql.se> wrote: > > On 30 Apr 2025, at 12:14, Peter Eisentraut <pe...@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 > >
v30-0001-Add-pg_get_process_memory_context-function.patch
Description: Binary data
