Re: Redesign per-backend statistics

Hi,

On 2026-09-21 07:15:44 +0000, Bertrand Drouvot wrote:
> Subject: [PATCH v3 1/6] pgstat: add tests for per-backend statistics

Good idea to start with that.

> Add an isolation test for per-backend WAL statistics. Verify that SNAPSHOT, CACHE
> and NONE modes behave as expected.

This isn't really related to this patch, but I do wonder if we need to make
this somehow a bit more scalable for testing. If every stats thing has to test
all of these "manually", we have a test-scalability issue...

> Extend test_shm_mq with a shared-memory-only worker that reports WAL usage
> through a routine nonblocking flush and remains alive while the controlling
> backend verifies that the global WAL counters include it.

I'm not entirely sure what this really tests? Couldn't you just test this
e.g. for checkpointer, in a quarter of the lines at most?

> +++ b/src/test/isolation/specs/stats-per-backend.spec
> @@ -0,0 +1,123 @@
> +# Test snapshot, cache, and reset behavior for per-backend statistics.
> +#
> +# Session s2 generates and flushes WAL records, which provide a deterministic
> +# per-backend counter. Session s1 verifies that SNAPSHOT mode fixes all
> +# per-backend entries at the initial statistics snapshot, while CACHE mode
> +# fixes each entry on first access and refreshes it after the transaction
> +# ends. The final permutations verify reset timestamps after per-backend and
> +# shared resets.
> +
> +setup
> +{
> + CREATE TABLE stats_per_backend_data(id int);
> + CREATE TABLE stats_per_backend_saved(wal_records bigint);
> +}
> +
> +teardown
> +{
> + DROP TABLE stats_per_backend_data;
> + DROP TABLE stats_per_backend_saved;
> +}
> +
> +session s1
> +setup { SET stats_fetch_consistency = 'none'; }
> +
> +step s1_fetch_consistency_cache { SET stats_fetch_consistency = 'cache'; }
> +step s1_fetch_consistency_snapshot { SET stats_fetch_consistency = 'snapshot'; }
> +step s1_begin { BEGIN; }
> +step s1_commit { COMMIT; }
> +step s1_build_snapshot {
> + SELECT wal_records >= 0 AS snapshot_created FROM pg_stat_wal;
> +}
> +step s1_check_snapshot_backend {
> + SELECT wal_records = 0 AS snapshot_excludes_later_update
> + FROM pg_stat_get_backend_wal(
> + (SELECT pid FROM pg_stat_activity
> + WHERE application_name = 'isolation/stats-per-backend/s2'));
> +}

It's imo not particularly safe to assert 0, you can get on-access pruning or
hint-bit sets triggering WAL writes. I think you pretty much need to assert
that it's monotonically increasing, unfortunately.

> --- a/src/test/modules/test_misc/meson.build
> +++ b/src/test/modules/test_misc/meson.build
> @@ -24,6 +24,7 @@ tests += {
> 't/013_temp_obj_multisession.pl',
> 't/014_log_statement_max_length.pl',
> 't/015_temp_schema_exit_deferrable.pl',
> + 't/016_stats_snapshot.pl',
> ],
> # The injection points are cluster-wide, so disable installcheck
> 'runningcheck': false,
> diff --git a/src/test/modules/test_misc/t/016_stats_snapshot.pl b/src/test/modules/test_misc/t/016_stats_snapshot.pl
> new file mode 100644
> index 00000000000..43c9157bbb4
> --- /dev/null
> +++ b/src/test/modules/test_misc/t/016_stats_snapshot.pl
> @@ -0,0 +1,48 @@
> +# Copyright (c) 2026, PostgreSQL Global Development Group
> +
> +# Verify that a statistics snapshot excludes backends that started after the
> +# snapshot was created.
> +
> +use strict;
> +use warnings FATAL => 'all';
> +
> +use PostgreSQL::Test::Cluster;
> +use PostgreSQL::Test::Utils;
> +use Test::More;
> +
> +my $node = PostgreSQL::Test::Cluster->new('stats_snapshot');
> +$node->init;
> +$node->start;

Is this really worth creating another cluster for? That's not that cheap, and
our test codes really are exploding lately, largely due to creating lots of
tiny clusters that are just used for a tiny test.

> From 94c8483511c29e5837fbd299294ecb42d8cac265 Mon Sep 17 00:00:00 2001
> From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
> Date: Wed, 29 Jul 2026 14:11:27 +0000
> Subject: [PATCH v3 2/6] pgstat: add new infrastructure for per-backend
> statistics
>
> Add a new infrastructure for per-backend statistics that keep one shared entry
> per live backend while keeping their existing shared data as global stats for
> exited backends.
>
> Define a common dshash entry header containing the ProcNumber key, BackendType,
> and content LWLock. Extend PgStat_KindInfo with per-backend statistics related
> informations.
>
> At shared-memory initialization, create a dshash for every participating kind.

> +/* -------------------------------------------------------------------------
> + *
> + * pgstat_per_backend.c
> + * Generic infrastructure for per-backend statistics.
> + *
> + * This file manages the dedicated per-kind dshashes used for per-backend
> + * statistics, including entry creation, fetching, snapshots, transfer to
> + * global statistics, and removal.

It's not entirely obvious to me that a hash table is needed here. If I
understand correctly, this just going to be used for per-backend versions of
fixed stats. Which means we could just as well allocate all the memory in
statically allocated shared memory at server start and have the per-backend
stats be accessible by nothing more than an array access?

Given this is going to be used for stuff like WAL accesses etc, which just
about every backend will participate, I think the dshash overhead will just
cost (CPU, complexity, even memory), without gaining you meaningful memory
savings in realistic cases.

Greetings,

Andres Freund