On Mon, Oct 21, 2024 at 11:54:21PM +0530, Rahila Syed wrote: > On the other hand, [2] provides the statistics for all backends but logs > them in a file, which may not be convenient for quick access.
To be precise, pg_log_backend_memory_contexts() pushes the memory context stats to LOG_SERVER_ONLY or stderr, hence this is appended to the server logs. > A fixed-size shared memory block, currently accommodating 30 records, > is used to store the statistics. This number was chosen arbitrarily, > as it covers all parent contexts at level 1 (i.e., direct children of the > top memory context) > based on my tests. > Further experiments are needed to determine the optimal number > for summarizing memory statistics. + * Statistics are shared via fixed shared memory which + * can hold statistics for 29 contexts. The rest of the [...] + MemoryContextInfo memctx_infos[30]; [...] + memset(&memCtxState->memctx_infos, 0, 30 * sizeof(MemoryContextInfo)); [...] + size = add_size(size, mul_size(30, sizeof(MemoryContextInfo))); [...] + memset(&memCtxState->memctx_infos, 0, 30 * sizeof(MemoryContextInfo)); [...] + memset(&memCtxState->memctx_infos, 0, 30 * sizeof(MemoryContextInfo)); This number is tied to MemoryContextState added by the patch. Sounds like this would be better as a constant properly defined rather than hardcoded in all these places. This would make the upper-bound more easily switchable in the patch. + Datum path[128]; + char type[128]; [...] + char name[1024]; + char ident[1024]; + char type[128]; + Datum path[128]; Again, constants. Why these values? You may want to use more #defines here. > Any additional statistics that exceed the shared memory capacity > are written to a file per backend in the PG_TEMP_FILES_DIR. The client > backend > first reads from the shared memory, and if necessary, retrieves the > remaining data from the file, > combining everything into a unified view. The files are cleaned up > automatically > if a backend crashes or during server restarts. Is the addition of the file to write any remaining stats really that useful? This comes with a heavy cost in the patch with the "in_use" flag, the various tweaks around the LWLock release/acquire protecting the shmem area and the extra cleanup steps required after even a clean restart. That's a lot of facility for this kind of information. Another thing that may be worth considering is to put this information in a DSM per the variable-size nature of the information, perhaps cap it to a max to make the memory footprint cheaper, and avoid all on-disk footprint because we don't need it to begin with as this is information that makes sense only while the server is running. Also, why the single-backend limitation? One could imagine a shared memory area indexed similarly to pgproc entries, that includes auxiliary processes as much as backends, so as it can be possible to get more memory footprints through SQL for more than one single process at one moment in time. If each backend has its own area of shmem to deal with, they could use a shared LWLock on the shmem area with an extra spinlock while the context data is dumped into memory as the copy is short-lived. Each one of them could save the information in a DSM created only when a dump of the shmem is requested for a given PID, for example. -- Michael
signature.asc
Description: PGP signature
