Re: per backend WAL statistics

On Mon, Feb 24, 2025 at 09:07:39AM +0000, Bertrand Drouvot wrote:
> Now that 2421e9a51d2 is in, let's resume working in this thread. PFA a rebase to
> make the CF bot happy. Nothing has changed since V7, V8 only removes "v7-0001" (
> as part of 2421e9a51d2), so that v8-000N is nothing but v7-000(N+1).

v7-0001 looks sensible, so does v7-0002 with the introduction of
PgStat_WalCounters to tackle the fact that backend statistics need
only one reset_timestamp shared across IO and WAL stats.

+/*
+ * To determine whether WAL usage happened.
+ */
+static bool
+pgstat_backend_wal_have_pending(void)
+{
+ return pgWalUsage.wal_records != prevBackendWalUsage.wal_records;
+}

Okay for this pending data check.

--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -53,6 +53,8 @@ pgstat_report_wal(bool force)
/* flush wal stats */
pgstat_flush_wal(nowait);

+ pgstat_flush_backend(nowait, PGSTAT_BACKEND_FLUSH_WAL);
+
/* flush IO stats */
pgstat_flush_io(nowait);

Fine to stick that into pgstat_report_wal(), which is used anywhere
else. pgstat_flush_wal() could be static in pgstat_wal.c?

+Datum
+pg_stat_get_backend_wal(PG_FUNCTION_ARGS)
+{
[...]
+ pid = PG_GETARG_INT32(0);
+ proc = BackendPidGetProc(pid);
+
+ /*
+ * This could be an auxiliary process but these do not report backend
+ * statistics due to pgstat_tracks_backend_bktype(), so there is no need
+ * for an extra call to AuxiliaryPidGetProc().
+ */
+ if (!proc)
+ PG_RETURN_NULL();
+
+ procNumber = GetNumberFromPGProc(proc);
+
+ beentry = pgstat_get_beentry_by_proc_number(procNumber);
+ if (!beentry)
+ PG_RETURN_NULL();
+
+ backend_stats = pgstat_fetch_stat_backend(procNumber);
+ if (!backend_stats)
+ PG_RETURN_NULL();
+
+ /* if PID does not match, leave */
+ if (beentry->st_procpid != pid)
+ PG_RETURN_NULL();
+
+ /* backend may be gone, so recheck in case */
+ if (beentry->st_backendType == B_INVALID)
+ PG_RETURN_NULL();

This is a block of code copy-pasted from pg_stat_get_backend_io().
This is complex, so perhaps it would be better to refactor that in a
single routine that returns PgStat_Backend? Then reuse the refactored
code in both pg_stat_get_backend_io() and the new
pg_stat_get_backend_wal().

+-- Test pg_stat_get_backend_wal (and make a temp table so our temp schema exists)
+SELECT wal_bytes AS backend_wal_bytes_before from pg_stat_get_backend_wal(pg_backend_pid()) \gset
+
CREATE TEMP TABLE test_stats_temp AS SELECT 17;
DROP TABLE test_stats_temp;
[...]
+SELECT pg_stat_force_next_flush();
+SELECT wal_bytes > :backend_wal_bytes_before FROM pg_stat_get_backend_wal(pg_backend_pid());

That should be stable, as we're guaranteed to have records here.
--
Michael