Re: Enhancing Memory Context Statistics Reporting

From: Rahila Syed <rahilasyed90(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Enhancing Memory Context Statistics Reporting
Date: 2025-03-20 07:39:45
Message-ID: CAH2L28sMyRh_ZomRxkx_RdaQoLcyGAwKCr1TSmrVudbbR_Q1eQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

>>
> >> + if (procNumber == MyProcNumber)
> >> + {
> >> + ereport(WARNING,
> >> + errmsg("cannot return statistics for local backend"),
> >> + errhint("Use pg_backend_memory_contexts view
> instead."));
> >> + PG_RETURN_NULL();
> >> + }
> >>
> >> Is it worth it to keep this restriction? Can we fetch info about
> >> local memory context for the sake of generality?
> >>
> >
> > I think that could be done, but using pg_backend_memory_context would
> > be more efficient in this case.
> >
>
> I have raised a similar concern before. Having two separate functions
> one for local backend and other for remote is going to be confusing.
> We should have one function doing both and renamed appropriately.
>
>
This is a separate concern from what has been raised by Alexander.
He has suggested removing the restriction and fetching local backend
statistics also
with the proposed function.
I've removed this restriction in the latest version of the patch. Now, the
proposed
function can be used to fetch local backend statistics too.

Regarding your suggestion on merging these functions, although they both
report memory
context statistics, they differ in how they fetch these statistics—locally
versus from dynamic
shared memory. Additionally, the function signatures are different: the
proposed function
takes three arguments (pid, get_summary, and num_tries), whereas
pg_get_backend_memory_contexts does not take any arguments. Therefore, I
believe
these functions can remain separate as long as we document their usages
correctly.

Please find attached rebased and updated patches. I have added some more
comments and
fixed an issue caused due to registering before_shmem_exit callback from
interrupt processing
routine. To address this issue, I am registering this callback in the
InitProcess() function.

This happened because interrupt processing could be triggered from a
PG_ENSURE_ERROR_CLEANUP block. This block operates under the assumption
that
the before_shmem_exit callback registered at the beginning of the block,
will be the last one
in the registered callback list at the end of the block, which would not be
the case if I register
before_shmem_exit callback in the interrupt handling routine.

Thank you,
Rahila Syed

Attachment Content-Type Size
v18-0002-Function-to-report-memory-context-statistics.patch application/octet-stream 54.3 KB
v18-0001-Preparatory-changes-for-reporting-memory-context-sta.patch application/octet-stream 4.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2025-03-20 08:35:42 Re: Parallel heap vacuum
Previous Message Nisha Moond 2025-03-20 07:05:53 Re: Conflict detection for multiple_unique_conflicts in logical replication