Re: Enhancing Memory Context Statistics Reporting

Hi,

Please see some responses below.

On Mon, Apr 7, 2025 at 9:13 PM Andres Freund <andres(at)anarazel(dot)de> 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 over time. Today I have actually very little idea
> about what memory is being used for across all connections, not to speak
> of a
> larger number of servers.
>
>
> > 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
> > @@ -84,6 +84,13 @@ AuxiliaryProcessMainCommon(void)
> > /* register a before-shutdown callback for LWLock cleanup */
> > before_shmem_exit(ShutdownAuxiliaryProcess, 0);
> >
> > + /*
> > + * The before shmem exit callback frees the DSA memory occupied by
> the
> > + * latest memory context statistics that could be published by
> this aux
> > + * proc if requested.
> > + */
> > + before_shmem_exit(AtProcExit_memstats_dsa_free, 0);
> > +
> > SetProcessingMode(NormalProcessing);
> > }
>
> How about putting it into BaseInit()? Or maybe just register it when its
> first used?
>
>
Problem with registering it when dsa is first used is that dsa is used in
an interrupt handler.
The handler could be called from the PG_ENSURE_ERROR_CLEANUP block. This
block
operates under the assumption that the before_shmem_exit callback
registered at the beginning,
will be the last one in the registered callback list at the end of the
block. However, this won't be
the case if a callback is registered from an interrupt handler called in
the
PG_ENSURE_ERROR_CLEANUP block.

>

I don't really understand why DSA_DEFAULT_INIT_SEGMENT_SIZE is

something that makes sense to use here?
>
>
To determine the memory limit per backend in multiples of
DSA_DEFAULT_INIT_SEGMENT_SIZE.
Currently it is set to 1 * DSA_DEFAULT_INIT_SEGMENT_SIZE.
Since a call to dsa_create would create a DSA segment of this size, I
thought it makes sense
to define a limit related to the segment size.

> > +/*
>
> + /* If the dsm mapping could not be found, attach to the area */
> > + if (dsm_seg != NULL)
> > + return;
>
> I don't understand what we do here with the dsm? Why do we not need
> cleanup
> if we are already attached to the dsm segment?
>

I am not expecting to hit this case, since we are always detaching from the
dsa.
This could be an assert but since it is a cleanup code, I thought returning
would be
a harmless step.

Thank you,
Rahila Syed