Horton

Re: per backend WAL statistics

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-01-08T06:21:26Z
Lists: pgsql-hackers 
On Tue, Jan 07, 2025 at 08:48:51AM +0000, Bertrand Drouvot wrote:
> Now that commit 9aea73fc61 added backend-level statistics to pgstats (and
> per backend IO statistics), we can more easily add per backend statistics.
> 
> Please find attached a patch to implement $SUBJECT.

I've looked at v1-0002 and v1-0001.

+static void
+pgstat_flush_io_entry(PgStat_EntryRef *entry_ref, bool nowait, bool need_lock)
 {
-	PgStatShared_Backend *shbackendioent;
-	PgStat_BackendPendingIO *pendingent;
+	PgStatShared_Backend *shbackendent;
+	PgStat_BackendPending *pendingent;
 	PgStat_BktypeIO *bktype_shstats;
+	PgStat_BackendPendingIO *pending_io;
 
-	if (!pgstat_lock_entry(entry_ref, nowait))
-		return false;
+	if (need_lock && !pgstat_lock_entry(entry_ref, nowait))
+		return;

The addition of need_lock at this level leads to a result that seems a
bit confusing, where pgstat_backend_flush_cb() passes "false" because
it locks the entry by itself as an effect of v1-0003 with the new area
for WAL.  Wouldn't it be cleaner to do an extra pgstat_[un]lock_entry
dance in pgstat_backend_flush_io() instead?  Another approach I can
think of that would be slightly cleaner to me is to pass a bits32 to a
single routine that would control if WAL stats, I/O stats or both
should be flushed, keeping pgstat_flush_backend() as name with an
extra argument to decide which parts of the stats should be flushed.

-PgStat_BackendPendingIO *
+PgStat_BackendPending *

This rename makes sense.

 #define PG_STAT_GET_WAL_COLS	9
 	TupleDesc	tupdesc;
 	Datum		values[PG_STAT_GET_WAL_COLS] = {0};
 	bool		nulls[PG_STAT_GET_WAL_COLS] = {0};

It feels unnatural to have a PG_STAT_GET_WAL_COLS while it would not
only relate to this function anymore.
--
Michael

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