Hi, Please find attached an updated patch. It contains the following changes.
1. It needed a rebase as highlighted by cfbot <https://cfbot.cputube.org/patch_5938.log>. The method for adding an LWLock was updated in commit-2047ad068139f0b8c6da73d0b845ca9ba30fb33d, so the patch has been adjusted to reflect this change. 2. Updated some comments to align with the latest patch design. 3. Eliminated an unnecessary assertion Thank you, Rahila Syed On Fri, Jul 11, 2025 at 9:01 PM Rahila Syed <rahilasye...@gmail.com> wrote: > 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 >> >>
v31-0001-Add-pg_get_process_memory_context-function.patch
Description: Binary data
