Re: Enhancing Memory Context Statistics Reporting

> On 30 Apr 2025, at 12:14, Peter Eisentraut 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 repl

Re: Enhancing Memory Context Statistics Reporting

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.

Re: Enhancing Memory Context Statistics Reporting

Hi, 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. Thank you, Rahila Syed 0001-Fix-t

Re: Enhancing Memory Context Statistics Reporting

> > > > That's not an argument against moving it to BaseInit() though, as that's > called before procsignal is even initialized and before signals are > unmasked. > Yes, OK. > I don't really understand why DSA_DEFAULT_INIT_SEGMENT_SIZE is > > > > something that makes sense to use here? > > > > >

Re: Enhancing Memory Context Statistics Reporting

On 8 Apr 2025, at 07:40, Rahila Syed wrote:  > Renamed both to use MemoryContextReporting* namespace, which leaves > MemoryContextReportingBackendState at an unwieldly long name.  I'm running out > of ideas on how to improve and it does make purpose quite explicit at least. How about MemoryConte

Re: Enhancing Memory Context Statistics Reporting

Hi, On 2025-04-08 01:17:17 +0200, Daniel Gustafsson wrote: > > On 7 Apr 2025, at 17:43, Andres Freund wrote: > > >> + /* > >> + * Hold the process lock to protect writes to process specific memory. Two > >> + * processes publishing statistics do not block each other. > >> + */ > > > > s/specifi

Re: Enhancing Memory Context Statistics Reporting

On 2025/04/09 6:27, Daniel Gustafsson wrote: On 8 Apr 2025, at 18:41, Fujii Masao wrote: I noticed that the third argument of pg_get_process_memory_contexts() is named "retries" in pg_proc.dat, while the documentation refers to it as "timeout". I've committed this patch as it was obvious

Re: Enhancing Memory Context Statistics Reporting

> On 8 Apr 2025, at 18:41, Fujii Masao wrote: > I noticed that the third argument of pg_get_process_memory_contexts() is named > "retries" in pg_proc.dat, while the documentation refers to it as "timeout". I've committed this patch as it was obviously correct, thanks! > Also, as I mentioned ear

Re: Enhancing Memory Context Statistics Reporting

> On 8 Apr 2025, at 18:41, Fujii Masao wrote: > On 2025/04/08 18:46, Daniel Gustafsson wrote: >>> On 8 Apr 2025, at 10:03, Daniel Gustafsson wrote: >>> There was a bug in the shmem init function which caused it to fail on >>> Windows, >>> the attached fixes that. >> With this building green in C

Re: Enhancing Memory Context Statistics Reporting

On 2025/04/08 18:46, Daniel Gustafsson wrote: On 8 Apr 2025, at 10:03, Daniel Gustafsson wrote: There was a bug in the shmem init function which caused it to fail on Windows, the attached fixes that. With this building green in CI over several re-builds, and another pass over the docs and

Re: Enhancing Memory Context Statistics Reporting

> On 8 Apr 2025, at 10:03, Daniel Gustafsson wrote: > There was a bug in the shmem init function which caused it to fail on Windows, > the attached fixes that. With this building green in CI over several re-builds, and another pass over the docs and code with pgindent etc done, I pushed this ear

Re: Enhancing Memory Context Statistics Reporting

Hi Daniel, Andres, > > > >> +} MemoryContextState; > > > > > > IMO that's too generic a name for something in a header. > > > > > >> +} MemoryContextId; > > > > > > This too. Particularly because MemoryContextData->ident exist but is > > > something different. > > > > Renamed both to use MemoryC

Re: Enhancing Memory Context Statistics Reporting

> On 7 Apr 2025, at 17:43, Andres Freund wrote: > > Hi, > > On 2025-04-07 15:41:37 +0200, Daniel Gustafsson wrote: >> I think this function can be a valuable debugging aid going forward. > > What I am most excited about for this is to be able to measure server-wide and > fleet-wide memory usage

Re: Enhancing Memory Context Statistics Reporting

Hi, On 2025-04-07 21:57:57 +0530, Rahila Syed wrote: > > > diff --git a/src/backend/postmaster/auxprocess.c > > b/src/backend/postmaster/auxprocess.c > > > index 4f6795f7265..d3b4df27935 100644 > > > --- a/src/backend/postmaster/auxprocess.c > > > +++ b/src/backend/postmaster/auxprocess.c > > > @@

Re: Enhancing Memory Context Statistics Reporting

Hi, Please see some responses below. On Mon, Apr 7, 2025 at 9:13 PM Andres Freund wrote: > Hi, > > On 2025-04-07 15:41:37 +0200, Daniel Gustafsson wrote: > > I think this function can be a valuable debugging aid going forward. > > What I am most excited about for this is to be able to measure s

Re: Enhancing Memory Context Statistics Reporting

Hi, On 2025-04-07 15:41:37 +0200, Daniel Gustafsson wrote: > I think this function can be a valuable debugging aid going forward. What I am most excited about for this is to be able to measure server-wide and fleet-wide memory usage over time. Today I have actually very little idea about what mem

Re: Enhancing Memory Context Statistics Reporting

Following up on some off-list comments, attached is a v26 with a few small last changes: * Improved documentation (docs and comments) * Fixed up Shmem sizing and init * Delayed registering to the shmem cleanup to get it earlier in cleanup * Renamed a few datastructures to improve readabili

Re: Enhancing Memory Context Statistics Reporting

Hi Daniel, > > After a bit more polish I landed with the attached, which I most likely > will go > ahead with after another round in CI. > Thank you for refining the code. The changes look good to me. Regression tests ran smoothly in parallel with the memory monitoring function, pgbench results

Re: Enhancing Memory Context Statistics Reporting

> On 2 Apr 2025, at 23:44, Daniel Gustafsson wrote: > I think this version is close to a committable state, will spend a little more > time testing, polishing and rewriting the commit message. I will also play > around with placement within the memory context code files to keep it from > making

Re: Enhancing Memory Context Statistics Reporting

> On 26 Mar 2025, at 11:34, Rahila Syed wrote: > + ereport(LOG, > + errmsg("hash table corrupted, can't construct path value")); > I know you switched from elog(LOG.. to ereport(LOG.. but I still think a LOG > entry stating corruption isn't helpful, it's not actionable for t

Re: Enhancing Memory Context Statistics Reporting

Hi Daniel, Thank you for your review. I have incorporated all your changes in v20 patches and ensured that the review comments corresponding to 0001 patch are included in that patch and not in 0002. > > +MEM_CTX_PUBLISH"Waiting for backend to publish memory information." > I wonder if this

Re: Enhancing Memory Context Statistics Reporting

> On 20 Mar 2025, at 08:39, Rahila Syed wrote: Thanks for the new version, I believe this will be a welcome tool in the debugging toolbox. I took a cleanup pass over the docs with among others the below changes: * You had broken the text in paragraphs, but without tags they are rendered a

Re: Enhancing Memory Context Statistics Reporting

Hi, >> > >> + if (procNumber == MyProcNumber) > >> + { > >> + ereport(WARNING, > >> + errmsg("cannot return statistics for local backend"), > >> + errhint("Use pg_backend_memory_contexts view > instead.")); > >> + PG_RETURN_NULL(); > >> + } > >> > >> I

Re: Enhancing Memory Context Statistics Reporting

On Mon, Mar 17, 2025 at 1:23 PM Rahila Syed wrote: > >> >> v17-0002-Function-to-report-memory-context-statistics.patch >> >> + if (procNumber == MyProcNumber) >> + { >> + ereport(WARNING, >> + errmsg("cannot return statistics for local backend"), >> + errhint(

Re: Enhancing Memory Context Statistics Reporting

Hi Alexander, Thank you for the review. > It looks like we're increasing *num_contexts twice per child memory > context. First, it gets increased with a recursive > MemoryContextStatsInternal() call, then by adding an ichild. I might > be wrong, but I think these calculations at least deserve

Re: Enhancing Memory Context Statistics Reporting

Hi, Rahila! On Thu, Mar 13, 2025 at 3:57 PM Rahila Syed wrote: > > Please find attached updated and rebased patches. It has the following changes > > 1. To prevent memory leaks, ensure that the latest statistics published by a > process > are freed before it exits. This can be achieved by callin

Re: Enhancing Memory Context Statistics Reporting

Hi, Please find attached updated and rebased patches. It has the following changes 1. To prevent memory leaks, ensure that the latest statistics published by a process are freed before it exits. This can be achieved by calling dsa_free in the before_shmem_exit callback. 2. Add a level column to m

Re: Enhancing Memory Context Statistics Reporting

Hi Daniel, Thanks for the rebase, a few mostly superficial comments from a first > read-through. > Thank you for your comments. > The documentation refers to attributes in the return row but the format of > that > row isn't displayed which makes following along hard. I think we should > include

Re: Enhancing Memory Context Statistics Reporting

> On 24 Feb 2025, at 13:46, Rahila Syed wrote: > PFA the updated and rebased patches. Thanks for the rebase, a few mostly superficial comments from a first read-through. I'll do some more testing and playing around with it for functional comments. + ... + child contexts' statistics, with num

Re: Enhancing Memory Context Statistics Reporting

> I think something is not quite right, because if I try running a simple > pgbench script that does pg_get_process_memory_contexts() on PIDs of > random postgres process (just like in the past), I immediately get this: > > Thank you for testing. This issue occurs when a process that previously att

Re: Enhancing Memory Context Statistics Reporting

On 2/20/25 14:26, Rahila Syed wrote: > Hi, > > Please find attached the updated patches after some cleanup and test > fixes.   > > Thank you, > Rahila Syed > > On Tue, Feb 18, 2025 at 6:35 PM Rahila Syed > wrote: > > Hi, > > > Thanks for updating th

Re: Enhancing Memory Context Statistics Reporting

Hi, Please find attached the updated patches after some cleanup and test fixes. Thank you, Rahila Syed On Tue, Feb 18, 2025 at 6:35 PM Rahila Syed wrote: > Hi, > >> >> Thanks for updating the patch! >> >> The below comments would be a bit too detailed at this stage, but I’d >> like to share th

Re: Enhancing Memory Context Statistics Reporting

Hi, > > Thanks for updating the patch! > > The below comments would be a bit too detailed at this stage, but I’d > like to share the points I noticed. > > Thanks for sharing the detailed comments. I have incorporated some of them into the new version of the patch. I will include the rest when I re

Re: Enhancing Memory Context Statistics Reporting

On 2025-02-03 21:47, Rahila Syed wrote: Hi, Just idea; as an another option, how about blocking new requests to the target process (e.g., causing them to fail with an error or returning NULL with a warning) if a previous request is still pending? Users can simply retry the request if it

Re: Enhancing Memory Context Statistics Reporting

Hi, > > > > Just idea; as an another option, how about blocking new requests to > > the target process (e.g., causing them to fail with an error or > > returning NULL with a warning) if a previous request is still > pending? > > Users can simply retry the request if it fails. IMO

Re: Enhancing Memory Context Statistics Reporting

Hi, On Sat, Jan 25, 2025 at 3:50 AM Tomas Vondra wrote: > > > On 1/24/25 14:47, Rahila Syed wrote: > > > > Hi, > > > > > > Just idea; as an another option, how about blocking new requests to > > the target process (e.g., causing them to fail with an error or > > returning NULL with a

Re: Enhancing Memory Context Statistics Reporting

On 1/24/25 14:47, Rahila Syed wrote: > > Hi, > > > Just idea; as an another option, how about blocking new requests to > the target process (e.g., causing them to fail with an error or > returning NULL with a warning) if a previous request is still pending? > Users can simply

Re: Enhancing Memory Context Statistics Reporting

Hi, > > Just idea; as an another option, how about blocking new requests to > the target process (e.g., causing them to fail with an error or > returning NULL with a warning) if a previous request is still pending? > Users can simply retry the request if it fails. IMO failing quickly > seems prefe

Re: Enhancing Memory Context Statistics Reporting

On 2025/01/21 20:27, Rahila Syed wrote: Hi Tomas,  I've tried the pgbench test again, to see if it gets stuck somewhere, and I'm observing this on a new / idle cluster: $ pgbench -n -f test.sql -P 1 test -T 60 pgbench (18devel) progress: 1.0 s, 1647.9 tps, lat 0.604

Re: Enhancing Memory Context Statistics Reporting

Hi Tomas, I've tried the pgbench test > again, to see if it gets stuck somewhere, and I'm observing this on a > new / idle cluster: > > $ pgbench -n -f test.sql -P 1 test -T 60 > pgbench (18devel) > progress: 1.0 s, 1647.9 tps, lat 0.604 ms stddev 0.438, 0 failed > progress: 2.0 s, 1374.3 tps, la

Re: Enhancing Memory Context Statistics Reporting

Hi Tomas, Thank you for the review. On Tue, Jan 7, 2025 at 2:32 AM Tomas Vondra wrote: > Hi Rahila, > > Thanks for the updated and rebased patch. I've tried the pgbench test > again, to see if it gets stuck somewhere, and I'm observing this on a > new / idle cluster: > > $ pgbench -n -f test.sq

Re: Enhancing Memory Context Statistics Reporting

On 2025/01/08 21:03, Rahila Syed wrote: I have not been able to reproduce this issue. Could you please clarify which process you ran |pg_get_process_memory_context()| on, with the interval of 0.1? I used the following query for testing: =# SELECT count(*) FROM pg_stat_activity, pg_get_proc

Re: Enhancing Memory Context Statistics Reporting

Hi Fujii-san, Thank you for testing the feature. > Issue 1: Error with pg_get_process_memory_contexts() > When I used pg_get_process_memory_contexts() on the PID of a backend > process > that had just caused an error but hadn’t rolled back yet, > the following error occurred: > >Session 1 (P

Re: Enhancing Memory Context Statistics Reporting

Hi Rahila, Thanks for the updated and rebased patch. I've tried the pgbench test again, to see if it gets stuck somewhere, and I'm observing this on a new / idle cluster: $ pgbench -n -f test.sql -P 1 test -T 60 pgbench (18devel) progress: 1.0 s, 1647.9 tps, lat 0.604 ms stddev 0.438, 0 failed pr

Re: Enhancing Memory Context Statistics Reporting

On 2025/01/06 22:16, Rahila Syed wrote: PFA the patch with above updates. Thanks for updating the patch! I like this feature. I tested this feature and encountered two issues: Issue 1: Error with pg_get_process_memory_contexts() When I used pg_get_process_memory_contexts() on the PID of a

Re: Enhancing Memory Context Statistics Reporting

Hi Torikoshia, Thank you for the review. > > >=# select path FROM pg_get_process_memory_contexts('20271', false); >path >--- > {0} > {0,1} > {0,2} > .. > > =# select path from pg_backend_memory_contexts; > path >--- > {1} > {1,2} > {1,3} >

Re: Enhancing Memory Context Statistics Reporting

Hi Tomas, > > I'd just remove that. I agree it might have been useful with the single > chunk of shared memory, but I think with separate chunks it's not very > useful. And if we can end up with multiple processed getting the same > pgprocno I guess we have way bigger problems, this won't fix tha

Re: Enhancing Memory Context Statistics Reporting

Hi, Thanks for updating the patch and here are some comments: 'path' column of pg_get_process_memory_contexts() begins with 0, but that column of pg_backend_memory_contexts view begins with 1: =# select path FROM pg_get_process_memory_contexts('20271', false); path --- {0} {0,

Re: Enhancing Memory Context Statistics Reporting

Hi Rahila, Thanks for working on this. I've wanted something like this a number of times to replace my current method of attaching gdb like everyone else I suppose. I have a question / suggestion about the interface. +Datum +pg_get_process_memory_contexts(PG_FUNCTION_ARGS) +{ +int p

Re: Enhancing Memory Context Statistics Reporting

On 12/3/24 20:09, Rahila Syed wrote: > > Hi, >   > > > > >     4) I wonder if the function needs to return PID. I mean, the > caller > >     knows which PID it is for, so it seems rather unnecessary. > > > > Perhaps it can be used to ascertain that the information indeed >

Re: Enhancing Memory Context Statistics Reporting

Hi, > > > > 4) I wonder if the function needs to return PID. I mean, the caller > > knows which PID it is for, so it seems rather unnecessary. > > > > Perhaps it can be used to ascertain that the information indeed belongs > to > > the requested pid. > > > > I find that a bit ... suspicio

Re: Enhancing Memory Context Statistics Reporting

On 11/29/24 00:23, Rahila Syed wrote: > Hi Tomas, > > Thank you for the review. > > > > 1) I read through the thread, and in general I agree with the reasoning > for removing the file part - it seems perfectly fine to just dump as > much as we can fit into a buffer, and then summari

Re: Enhancing Memory Context Statistics Reporting

Hi Tomas, Thank you for the review. > > > 1) I read through the thread, and in general I agree with the reasoning > for removing the file part - it seems perfectly fine to just dump as > much as we can fit into a buffer, and then summarize the rest. But do we > need to invent a "new" limit here?

Re: Enhancing Memory Context Statistics Reporting

Hi, I took a quick look at the patch today. Overall, I think this would be very useful, I've repeatedly needed to inspect why a backend uses so much memory, and I ended up triggering MemoryContextStats() from gdb. This would be more convenient / safer. So +1 to the patch intent. A couple review

Re: Enhancing Memory Context Statistics Reporting

On Fri, Nov 22, 2024 at 6:33 PM Rahila Syed wrote: > > Hi, > >> How does the process know that the client backend has finished reading >> stats and it can be refreshed? What happens, if the next request for >> memory context stats comes before first requester has consumed the >> statistics it requ

Re: Enhancing Memory Context Statistics Reporting

Hi, How does the process know that the client backend has finished reading > stats and it can be refreshed? What happens, if the next request for > memory context stats comes before first requester has consumed the > statistics it requested? > > A process that's copying its statistics does not nee

Re: Enhancing Memory Context Statistics Reporting

On Wed, Nov 20, 2024 at 2:39 PM Rahila Syed wrote: > > Hi, > >> To achieve both completeness and avoid writing to a file, I can consider >> displaying the numbers for the remaining contexts as a cumulative total >> at the end of the output. >> >> Something like follows: >> ``` >> postgres=# select

Re: Enhancing Memory Context Statistics Reporting

Hi, To achieve both completeness and avoid writing to a file, I can consider > displaying the numbers for the remaining contexts as a cumulative total > at the end of the output. > > Something like follows: > ``` > postgres=# select * from pg_get_process_memory_contexts('237244', false); >

Re: Enhancing Memory Context Statistics Reporting

Hi, On Thu, Nov 14, 2024 at 5:18 PM Alvaro Herrera wrote: > On 2024-Nov-14, Michael Paquier wrote: > > > Already mentioned previously at [1] and echoing with some surrounding > > arguments, but I'd suggest to keep it simple and just remove entirely > > the part of the patch where the stats infor

Re: Enhancing Memory Context Statistics Reporting

On 2024-Nov-14, Michael Paquier wrote: > Already mentioned previously at [1] and echoing with some surrounding > arguments, but I'd suggest to keep it simple and just remove entirely > the part of the patch where the stats information gets spilled into > disk. With more than 6000-ish context info

Re: Enhancing Memory Context Statistics Reporting

On Wed, Nov 13, 2024 at 01:00:52PM +0530, Rahila Syed wrote: > I used one DSA area per process to share statistics. Currently, > the size limit for each DSA is 16 MB, which can accommodate > approximately 6,700 MemoryContextInfo structs. Any additional > statistics will spill over to a file. I opte

Re: Enhancing Memory Context Statistics Reporting

Hi, Thank you for the review. > > Hmm, would it make sene to use dynamic shared memory for this? The > publishing backend could dsm_create one DSM chunk of the exact size that > it needs, pass the dsm_handle to the consumer, and then have it be > destroy once it's been read. That way you don't

Re: Enhancing Memory Context Statistics Reporting

Hi, On 2024-10-26 16:14:25 +0200, Alvaro Herrera wrote: > > A fixed-size shared memory block, currently accommodating 30 records, > > is used to store the statistics. > > Hmm, would it make sene to use dynamic shared memory for this? +1 > The publishing backend could dsm_create one DSM chunk o

Re: Enhancing Memory Context Statistics Reporting

On 2024-Oct-21, Rahila Syed wrote: > I propose enhancing memory context statistics reporting by combining > these capabilities and offering a view of memory statistics for all > PostgreSQL backends and auxiliary processes. Sounds good. > A fixed-size shared memory block, currently a

Re: Enhancing Memory Context Statistics Reporting

Hi Rahila, I’ve spent some time reviewing the patch, and the review is still ongoing. Here are the comments I’ve found so far. 1. The tests are currently missing. Could you please add them? 2. I have some concerns regarding the function name ‘pg_get_remote_backend_memory_contexts’. Specifically,

Re: Enhancing Memory Context Statistics Reporting

to the local backend, restricting its use to PostgreSQL client backends only. 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. I propose enhancing memory context statistics reporting by combining these

Re: Enhancing Memory Context Statistics Reporting

stgreSQL logs. Although [1] offers detailed > > statistics, > > it is limited to the local backend, restricting its use to PostgreSQL > > client backends only. > > On the other hand, [2] provides the statistics for all backends but > > logs them in a file, > > whic

Re: Enhancing Memory Context Statistics Reporting

Hi Michael, Thank you for the review. On Tue, Oct 22, 2024 at 12:18 PM Michael Paquier wrote: > 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 acc

Re: Enhancing Memory Context Statistics Reporting

logs them in a file, which may not be convenient for quick access. I propose enhancing memory context statistics reporting by combining these capabilities and offering a view of memory statistics for all PostgreSQL backends and auxiliary processes. Thanks for working on this! I originally tried

Re: Enhancing Memory Context Statistics Reporting

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

Enhancing Memory Context Statistics Reporting

convenient for quick access. I propose enhancing memory context statistics reporting by combining these capabilities and offering a view of memory statistics for all PostgreSQL backends and auxiliary processes. Attached is a patch that implements this functionality. It introduces a SQL function that