Re: Add tracking of backend memory allocated to pg_stat_activity

At Wed, 31 Aug 2022 12:05:55 -0500, Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote in
> On Wed, Aug 31, 2022 at 12:03:06PM -0400, Reid Thompson wrote:
> > Hi Hackers,
> >
> > Attached is a patch to
> > Add tracking of backend memory allocated to pg_stat_activity
>
> > + proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
>
> In the past, there was concern about making pg_stat_activity wider by
> adding information that's less-essential than what's been there for
> years. This is only an int64, so it's not "wide", but I wonder if
> there's another way to expose this information? Like adding backends to

The view looks already too wide to me. I don't want the numbers for
metrics are added to the view.

> pg_get_backend_memory_contexts() , maybe with another view on top of
> that ?

+1

> + * shown allocated in pgstat_activity when the creator destroys the
>
> pg_stat
>
> > + * Posix creation calls dsm_impl_posix_resize implying that resizing
> > + * occurs or may be added in the future. As implemented
> > + * dsm_impl_posix_resize utilizes fallocate or truncate, passing the
> > + * whole new size as input, growing the allocation as needed * (only
> > + * truncate supports shrinking). We update by replacing the * old
>
> wrapping caused extraneous stars
>
> > + * Do not allow backend_mem_allocated to go below zero. ereport if we
> > + * would have. There's no need for a lock around the read here asit's
>
> as it's
>
> > + ereport(LOG, (errmsg("decrease reduces reported backend memory allocated below zero; setting reported to 0")));
>
> errmsg() doesn't require the outside paranthesis since a couple years
> ago.

+1

> > + /*
> > + * Until header allocation is included in context->mem_allocated cast to
> > + * slab and decrement the headerSize
>
> add a comma before "cast" ?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center