Horton

Re: per backend WAL statistics

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, pgsql-hackers@lists.postgresql.org
Date: 2025-01-16T16:38:47Z
Lists: pgsql-hackers 
Hi,

On 2025-01-16 15:59:31 +0000, Bertrand Drouvot wrote:
> On Wed, Jan 15, 2025 at 03:11:32PM +0900, Michael Paquier wrote:
> > On Fri, Jan 10, 2025 at 09:40:38AM +0000, Bertrand Drouvot wrote:
> > + * WAL pending statistics are incremented inside a critical section
> > + * (see XLogWrite()), so we can't use pgstat_prep_pending_entry() and we rely on
> > + * PendingBackendWalStats instead.
> > + */
> > +extern PGDLLIMPORT PgStat_PendingWalStats PendingBackendWalStats;
> > 
> > Hmm.  This makes me wonder if we should rethink a bit the way pending
> > entries are retrieved and if we should do it beforehand for the WAL
> > paths to avoid allocations in some critical sections.  Isn't that also
> > because we avoid calling pgstat_prep_backend_pending() for the I/O
> > case as only backends are supported now, discarding cases like the
> > checkpointer where I/O could happen in a critical path?  As a whole,
> > the approach taken by the patch is not really consistent with the
> > rest.

> I agree that's better to have a generic solution and to be consistent with
> the other variable-numbered stats. 
> 
> The attached is implementing in 0001 the proposition done in [1], i.e:
> 
> 1. It adds a new allow_critical_section to PgStat_KindInfo for pgstats kinds
> 2. It ensures to set temporarly allowInCritSection to true when needed
>
> Note that for safety reason 0001 does set allowInCritSection back to false
> unconditionally (means not checking again for allow_critical_section).

This is a preposterously bad idea.  The restriction to not allocate memory in
critical sections exists for a reason, why on earth should this code be
allowed to just opt out of the restriction of not allowing memory allocations
in critical sections?

The only cases where we can somewhat safely allocate memory in critical
sections is when using memory contexts with pre-reserved memory, where there's
a pretty low bound on how much memory is going to be needed. E.g. logging a
message inside a critical section, where elog.c can reset ErrorContext
afterwards.

Greetings,

Andres Freund

Commits

  1. Fix use-after-free in pgstat_fetch_stat_backend_by_pid()

  2. Remove initialization from PendingBackendStats

  3. Add WAL data to backend statistics

  4. Improve check for detection of pending data in backend statistics

  5. Fix some gaps in pg_stat_io with WAL receiver and WAL summarizer

  6. Handle auxiliary processes in SQL functions of backend statistics

  7. Invent pgstat_fetch_stat_backend_by_pid()

  8. Refactor code of pg_stat_get_wal() building result tuple

  9. Adding new PgStat_WalCounters structure in pgstat.h

  10. Remove pgstat_flush_wal()

  11. Refactor some code related to backend statistics

  12. Add backend-level statistics to pgstats