Redesign per-backend statistics
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253299psql -h localhost -U postgresBuilt from patchset v7 (message #7), October 12, 2026 at 02:26 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253299_7 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253299_7 && git checkout t253299_7Patchset v7 (message #7) is on t253299_7
Hi hackers,
The current per-backend statistics design excludes several process types and
duplicates accounting and storage: WAL, lock, and IO activity is reported both to
the global statistics and to PGSTAT_KIND_BACKEND.
This duplicated work was raised as a concern in [1]/messages/by-id/7fhpds4xqk6bnudzmzkqi33pinsxammpljwde5gfkjdygvejrj@ojkzfr7dxkmm, and its cost would grow as
more statistics acquire per-backend reporting.
Andres proposed a new design in [2]/messages/by-id/et272fdhdx6yphlgzvrgsf7bgwnf3vqciwp4gxqubro42uaflp@ohslaocvwgvi:
"
I think what we'd want is rather something where for each applicable stats kind
we have a shared counter for all exited backends and then per-backend counters
for live backends, with helpers to aggregate the exited + live stats to a total
"
and this is what the attached patch series is implementing.
Its main benefits are:
1/ each counter is reported and stored once, rather than in both the fixed global
statistics and PGSTAT_KIND_BACKEND.
2/ WAL, lock, and IO flushes update a per backend entry without acquiring
the corresponding global statistics lock or performing a hash lookup. This removes
global lock contention from the flush path.
3/ WAL, lock, and IO statistics have independent storage and locking instead of
sharing one PGSTAT_KIND_BACKEND entry.
4/ per-backend statistics become available for auxiliary processes previously
excluded by the backend type filtering.
There are 2 tradeoff though:
1/ fetching global statistics now requires combining all live entries.
2/ a shared statistics reset now clears both the global stats and all live entries.
Otherwise, values from live entries would immediately reappear in the global view.
The first tradeoff moves work from the frequent flush path to the comparatively
infrequent query path. The second is a documented behavior change.
The patch series is organized that way:
0001: add tests for per-backend statistics
It adds new tests that will serve as compatibility coverage for the redesign.
It could be applied while we are discussing the other patches.
0002: add new infrastructure for per-backend statistics
It introduces the common per-backend entry header, the kind metadata needed to
describe per-backend storage, and backend-local state holding the attached dshash
and cached entry pointer. It provides the common operations for creating and
attaching the hashes, creating and caching the current process's entries, fetching
an individual entry, constructing consistent snapshots, transferring entries into
global statistics, removing entries, and the accumulation at clean server shutdown.
No statistics kind registers per-backend metadata in this patch.
0003 moves WAL statistics to the infrastructure introduced by 0002.
0004 performs the corresponding conversion for lock statistics.
0005 performs the corresponding conversion for IO statistics. Once WAL, lock,
and IO have moved, PGSTAT_KIND_BACKEND contains no data, so this patch also removes
that kind and pgstat_backend.c, together with their obsolete infrastructure.
Design explanation for the new hashes:
- a fixed array indexed by ProcNumber would avoid hash operations, but it would
reserve shared memory for every possible process slot and every participating kind.
Queries could also have to inspect unused slots. The dshash allocates entries
for processes that actually exist and lets queries iterate those entries directly.
- reusing the current variable numbered statistics hash would require aggregate
queries to scan unrelated statistics entries or require another structure for
enumerating only the live entries of that kind. A dedicated per kind hash provides
that enumeration directly.
Remark:
The patch series limits this new infrastructure to built in fixed numbered
statistics kinds as this is the only use case we have had so far. We could extend
to variable ones later on if needed.
[1]: /messages/by-id/7fhpds4xqk6bnudzmzkqi33pinsxammpljwde5gfkjdygvejrj@ojkzfr7dxkmm
[2]: /messages/by-id/et272fdhdx6yphlgzvrgsf7bgwnf3vqciwp4gxqubro42uaflp@ohslaocvwgvi
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
t253299_1v1-0001-pgstat-add-tests-for-per-backend-statistics.patchtext/x-diff; charset=us-asciiDownload+434-1
v1-0002-pgstat-add-new-infrastructure-for-per-backend-sta.patchtext/x-diff; charset=us-asciiDownload+599-1
v1-0003-pgstat-move-WAL-statistics-to-new-per-backend-inf.patchtext/x-diff; charset=us-asciiDownload+232-111
v1-0004-pgstat-move-Lock-statistics-to-new-per-backend-in.patchtext/x-diff; charset=us-asciiDownload+218-124
v1-0005-pgstat-move-IO-statistics-to-new-per-backend-infr.patchtext/x-diff; charset=us-asciiDownload+282-541
On Mon, Aug 03, 2026 at 02:20:16PM +0000, Bertrand Drouvot wrote:
0001: add tests for per-backend statistics
It adds new tests that will serve as compatibility coverage for the redesign.
It could be applied while we are discussing the other patches.
That seems useful on its own. Will look at that in details first.
0002: add new infrastructure for per-backend statistics
It introduces the common per-backend entry header, the kind metadata needed to
describe per-backend storage, and backend-local state holding the attached dshash
and cached entry pointer. It provides the common operations for creating and
attaching the hashes, creating and caching the current process's entries, fetching
an individual entry, constructing consistent snapshots, transferring entries into
global statistics, removing entries, and the accumulation at clean server shutdown.
Okay, so if I get it right when a backend start we get a new reference
to the new dshash you are introducing, then each backend uses this
reference to push its stats updates. This is moving the cost of
aggregating the data when querying the data of each backend for WAL,
IO and lock stats, rather than do twice the aggregate for the central
WAL/IO/lock data plus the backend counterpart on HEAD.
Then the reason why you are using a new dshash to keep track of the
backend data is cost: you need to read all the backend-side data when
querying pg_stat_io, pg_stat_wal or pg_stat_lock, and you don't want
to trigger a full sequential scan of the dshash.
Hmm. First, do we need a dshash at all? The number of backends is
fixed at startup so we could use a set of arrays instead for a cheaper
access (lock, WAL and IO), allocated in shmem? The locking gets fuzzy
then:
- Data is written by each backend.
- Data could be read by anything else.
Using one LWLock for each backend sounds costly just for more
correctness with the stats, and we don't have that many writes anyway?
Second, you may be interested in this patch:
/messages/by-id/CAA5RZ0supQBxSkh=CWB39=j+cL3hHcLPki3tcBk0B1r4fesg_g@mail.gmail.com
This is for PGSS, but could be applied to your patch set. The idea is
simple: keep the stats kind for backends, but register a dedicated
dshash for it rather than having more dsa facilities in the area of
pgstat.c.
pgstat.c becomes much larger, with a bunch of knowledge now related to
backends. Would it make sense to invent a pgstat_snapshot.c where the
existing snapshot-related code now in pgstat.c could be moved? Or
perhaps pgstat_backend.c should be kept, with its snapshot code there?
Keeping all that knowledge in pgstat.c is not really nice, at least
IMV.
No statistics kind registers per-backend metadata in this patch.
0003 moves WAL statistics to the infrastructure introduced by 0002.
0004 performs the corresponding conversion for lock statistics.
0005 performs the corresponding conversion for IO statistics. Once WAL, lock,
and IO have moved, PGSTAT_KIND_BACKEND contains no data, so this patch also removes
that kind and pgstat_backend.c, together with their obsolete infrastructure.
0003~0005 are a bit boring, in the good sense. Perhaps you should
split 0005 into a 0005 for the IO move to these new APIs and a 0006 to
remove the stats kind. Based on my other comments, I am not actually
convinced that we absolutely have to drop PGSTAT_KIND_BACKEND, quite
the contrary.
--
Michael
Hi,
On Fri, Aug 07, 2026 at 10:39:04AM +0900, Michael Paquier wrote:
On Mon, Aug 03, 2026 at 02:20:16PM +0000, Bertrand Drouvot wrote:
0001: add tests for per-backend statistics
It adds new tests that will serve as compatibility coverage for the redesign.
It could be applied while we are discussing the other patches.That seems useful on its own. Will look at that in details first.
Thanks!
0002: add new infrastructure for per-backend statistics
Okay, so if I get it right when a backend start we get a new reference
to the new dshash you are introducing, then each backend uses this
reference to push its stats updates.
Yeah, that's the idea. Each process creates and caches its entries during
initialization. Flushes then use the cached pointers, without a hash lookup.
Only the global WAL, Lock and I/O queries need to scan and aggregate all live
entries.
This is moving the cost of
aggregating the data when querying the data of each backend for WAL,
IO and lock stats, rather than do twice the aggregate for the central
WAL/IO/lock data plus the backend counterpart on HEAD.
Right, with one precision: the aggregation cost moves to queries of the global
views, not to queries of an individual backend. A per-backend fetch still looks
up only one ProcNumber entry. On current master, a flush updates both the fixed
global statistics and PGSTAT_KIND_BACKEND. With the new design it updates
only the live entry, and a global query combines the fixed accumulator with all
live entries.
Then the reason why you are using a new dshash to keep track of the
backend data is cost: you need to read all the backend-side data when
querying pg_stat_io, pg_stat_wal or pg_stat_lock, and you don't want
to trigger a full sequential scan of the dshash.Hmm. First, do we need a dshash at all? The number of backends is
fixed at startup so we could use a set of arrays instead for a cheaper
access (lock, WAL and IO), allocated in shmem?
Yeah, that was my first comment in the "Design explanation for the new hashes"
section of my first email in this thread:
an earlier POC version used arrays indexed by ProcNumber. The main concern was
reserving storage for every possible process slot and every kind, even with
few active processes. With that, max_connections=10000 would reserve about 33.5MB
for the three kinds. Queries would also need to scan unused slots or use another
structure to track active ones.
I don't think that the fixed allocation would be the right design, particularly
because it grows with MaxBackends even when most slots are unused. Since v1
already avoids hash lookups on the flush path, I don't think the simpler access
justifies that memory cost.
Using one LWLock for each backend sounds costly just for more
correctness with the stats, and we don't have that many writes anyway?
I think that an array would not remove the need for content synchronization. The
dshash partition lock protects the entry lifetime, but the owner updates its
cached entry without holding that lock. Queries read the entry, while shared
and per-backend resets can modify it from another process. In particular, a
concurrent reset with a flush could lose counters.
v1 uses one LWLock per kind and live process for that.
Second, you may be interested in this patch:
/messages/by-id/CAA5RZ0supQBxSkh=CWB39=j+cL3hHcLPki3tcBk0B1r4fesg_g@mail.gmail.com
This is for PGSS, but could be applied to your patch set. The idea is
simple: keep the stats kind for backends, but register a dedicated
dshash for it rather than having more dsa facilities in the area of
pgstat.c.
Yeah, I started looking at Sami's patch yesterday, after you mentioned it to me
off-list.
PGSTAT_KIND_BACKEND is variable-numbered, so it could directly use own_hash.
Also, keeping PGSTAT_KIND_BACKEND and using own_hash could still eliminate duplicate
accounting if routine flushes updated only its live entry, while the fixed WAL,
Lock and I/O structures retained on exit transferred statistics.
That said, I don't think keeping the combined kind is a good fit for this design:
A WAL snapshot or reset would acquire the same entry lock used by Lock and I/O
flushes. Exit and ProcNumber reuse would also have to coordinate one combined
entry with the three fixed accumulators and their locks.
In the new design, each fixed kind owns both parts of its statistics: its fixed
accumulator and its live per-process entries. A snapshot, reset, or transfer
therefore involves only that kind's lock and hash.
IIUC, own_hash would only change where the generic variable-statistics entries
are stored: it would retain their refcount, drop and garbage collection machinery.
V1 instead uses ProcNumber keyed entries with a process lifetime lifecycle, so
that machinery is not needed.
One detail is that v1 does not add another DSA: all three hashes share the
existing pgstat DSA, while Sami's patch creates a dedicated DSA for an own_hash
kind.
So, while own_hash could be used to implement a combined one hash alternative,
I don't think it provides the same per-kind isolation or ownership as v1.
pgstat.c becomes much larger, with a bunch of knowledge now related to
backends.
Agreed. I wonder if introducing pgstat_per_backend.c wouldn't make more sense.
The added code handles entry creation, fetching, transfer and removal in addition
to snapshots, so pgstat_snapshot.c seems too narrow.
0003~0005 are a bit boring, in the good sense. Perhaps you should
split 0005 into a 0005 for the IO move to these new APIs and a 0006 to
remove the stats kind.
Yeah, good point. Moving the removal into 0006 would make more sense.
Based on my other comments, I am not actually
convinced that we absolutely have to drop PGSTAT_KIND_BACKEND, quite
the contrary.
Right, as mentioned above, it could be retained, but I don't think it should be.
It would keep WAL, Lock and I/O coupled through one entry and content lock, while
exit and ProcNumber reuse would need to coordinate that entry with three fixed
accumulators. Every additional per-backend statistic would then add more contention.
In the proposed design, each kind owns both its live entries and transferred
statistics.
Adding another per-backend statistic does not make it share an entry or content
lock with existing kinds.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Fri, Aug 07, 2026 at 12:31:47PM +0000, Bertrand Drouvot wrote:
On Fri, Aug 07, 2026 at 10:39:04AM +0900, Michael Paquier wrote:
an earlier POC version used arrays indexed by ProcNumber. The main concern was
reserving storage for every possible process slot and every kind, even with
few active processes. With that, max_connections=10000 would reserve about 33.5MB
for the three kinds. Queries would also need to scan unused slots or use another
structure to track active ones.
Hmm, okay, that feels like a waste. I doubt that anybody would use
that many max_connections, but if these are idle most of the time..
I don't think that the fixed allocation would be the right design, particularly
because it grows with MaxBackends even when most slots are unused. Since v1
already avoids hash lookups on the flush path, I don't think the simpler access
justifies that memory cost.
Accessing an array indexed by procnumber should be slightly cheaper
than a hash lookup when grabbing the stats of an individual backend,
as this is just a BackendPidGetProc() -> GetNumberFromPGProc() to get
a location.
Using one LWLock for each backend sounds costly just for more
correctness with the stats, and we don't have that many writes anyway?I think that an array would not remove the need for content
synchronization.
We would need some level of concurrent protection even with an array.
The dshash partition lock protects the entry lifetime, but the owner updates its
cached entry without holding that lock. Queries read the entry, while shared
and per-backend resets can modify it from another process. In particular, a
concurrent reset with a flush could lose counters.v1 uses one LWLock per kind and live process for that.
I can see that:
+pgstat_per_backend_snapshot(PgStat_Kind kind, dshash_table *hash, void *snap)
[...]
+ while ((entry = dshash_seq_next(&hstat)) != NULL)
+ {
+ LWLockAcquire(&entry->lock, LW_SHARED);
That's a sequential scan combined with potentially hundreds of LWLocks
acquired and released successivelly. That looks expensive here for a
single IO/lock/WAL data scan. That's the level of locking required
because a mutex cannot be hold while doing external calls, and here we
have one per_backend_acc_cb callback and one
pgstat_cache_per_backend_entry(). Not sure I like much this costly
locking level. I'm concerned by this cost.
--
Michael
Hi,
On Mon, Aug 10, 2026 at 01:38:31PM +0900, Michael Paquier wrote:
Accessing an array indexed by procnumber should be slightly cheaper
than a hash lookup when grabbing the stats of an individual backend,
as this is just a BackendPidGetProc() -> GetNumberFromPGProc() to get
a location.
Right, an array would make fetching an individual backend slightly cheaper.
My point was only that routine flushes use cached entry pointers and therefore
avoid hash lookups.
I can see that:
+pgstat_per_backend_snapshot(PgStat_Kind kind, dshash_table *hash, void *snap) [...] + while ((entry = dshash_seq_next(&hstat)) != NULL) + { + LWLockAcquire(&entry->lock, LW_SHARED);That's a sequential scan combined with potentially hundreds of LWLocks
acquired and released successivelly. That looks expensive here for a
single IO/lock/WAL data scan. That's the level of locking required
because a mutex cannot be hold while doing external calls, and here we
have one per_backend_acc_cb callback and one
pgstat_cache_per_backend_entry(). Not sure I like much this costly
locking level. I'm concerned by this cost.
Yeah, I benchmarked this against unpatched master (-O2 and assertions disabled).
Each sessions generated and flushed WAL/IO statistics, then remained connected
and idle. Then queried pg_stat_wal, pg_stat_lock and pg_stat_io:
Mean latency in ms:
400 backends 10000 backends
master v1 master v1
WAL 0.017 0.029 0.017 0.354
Lock 0.018 0.033 0.018 0.575
IO 0.069 0.143 0.069 2.847
Those are warmed, continuously repeated queries.
Now the impact:
- the extra timing is only when querying the corresponding global view
- the shared entry lock conflicts only with exclusive operations on the same
backend's entry for that statistics kind. The usual statistics flush uses
LWLockConditionalAcquire(), so it leaves counters pending rather than waiting.
Forced flushes, resets, and backend exit processing may wait, but other backends
can continue flushing their own entries.
FWIW, this kind of scan is not new:
- pg_locks walks the PGPROC slots and takes each live process's fpInfoLock in
shared mode.
- pg_stat_activity also performs a scan, although it uses a lockless changecount
and retry protocol rather than taking one LWLock per backend.
- the current statistics implementation with stats_fetch_consistency = snapshot
also scans the shared statistics dshash and takes each entry's content LWLock in
shared mode while copying it.
For comparison, select count(*) FROM pg_stat_activity took about 57 ms and select
count(*) FROM pg_locks took about 4ms, both with the same 10000 connections.
Given that the cost is still sub millisecond at hundreds of connections and a few
milliseconds at 10000 and given the impact mentioned above, I don't think this is
a practical blocker though. What do you think?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi,
On Mon, Aug 10, 2026 at 02:45:16PM +0000, Bertrand Drouvot wrote:
Given that the cost is still sub millisecond at hundreds of connections and a few
milliseconds at 10000 and given the impact mentioned above, I don't think this is
a practical blocker though. What do you think?
A rebase was needed, so while at it, I implemented what we have discussed so far,
means:
- moved the generic per-backend statistics infrastructure to a new pgstat_per_backend.c
file
- split the previous 0005 into the IO migration and the removal of PGSTAT_KIND_BACKEND
and pgstat_backend.c.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
t253299_6v2-0001-pgstat-add-tests-for-per-backend-statistics.patchtext/x-diff; charset=us-asciiDownload+434-1
v2-0002-pgstat-add-new-infrastructure-for-per-backend-sta.patchtext/x-diff; charset=us-asciiDownload+631-1
v2-0003-pgstat-move-WAL-statistics-to-new-per-backend-inf.patchtext/x-diff; charset=us-asciiDownload+232-111
v2-0004-pgstat-move-Lock-statistics-to-new-per-backend-in.patchtext/x-diff; charset=us-asciiDownload+218-124
v2-0005-pgstat-move-IO-statistics-to-new-per-backend-infr.patchtext/x-diff; charset=us-asciiDownload+268-286
v2-0006-pgstat-remove-PGSTAT_KIND_BACKEND.patchtext/x-diff; charset=us-asciiDownload+15-257
Hi,
On Thu, Aug 13, 2026 at 03:41:58PM +0000, Bertrand Drouvot wrote:
Hi,
On Mon, Aug 10, 2026 at 02:45:16PM +0000, Bertrand Drouvot wrote:
Given that the cost is still sub millisecond at hundreds of connections and a few
milliseconds at 10000 and given the impact mentioned above, I don't think this is
a practical blocker though. What do you think?A rebase was needed, so while at it, I implemented what we have discussed so far,
means:- moved the generic per-backend statistics infrastructure to a new pgstat_per_backend.c
file
- split the previous 0005 into the IO migration and the removal of PGSTAT_KIND_BACKEND
and pgstat_backend.c.
PFA another mandatory rebase (tiny change to test_misc/meson.build).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
t253299_7v3-0001-pgstat-add-tests-for-per-backend-statistics.patchtext/x-diff; charset=us-asciiDownload+434-1
v3-0002-pgstat-add-new-infrastructure-for-per-backend-sta.patchtext/x-diff; charset=us-asciiDownload+631-1
v3-0003-pgstat-move-WAL-statistics-to-new-per-backend-inf.patchtext/x-diff; charset=us-asciiDownload+232-111
v3-0004-pgstat-move-Lock-statistics-to-new-per-backend-in.patchtext/x-diff; charset=us-asciiDownload+218-124
v3-0005-pgstat-move-IO-statistics-to-new-per-backend-infr.patchtext/x-diff; charset=us-asciiDownload+268-286
v3-0006-pgstat-remove-PGSTAT_KIND_BACKEND.patchtext/x-diff; charset=us-asciiDownload+15-257
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.pg@gmail.com>
Date: Wed, 29 Jul 2026 14:11:27 +0000
Subject: [PATCH v3 2/6] pgstat: add new infrastructure for per-backend
statisticsAdd 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
On Mon, Sep 21, 2026 at 05:17:04AM -0400, Andres Freund wrote:
On 2026-09-21 07:15:44 +0000, Bertrand Drouvot wrote:
+/* ------------------------------------------------------------------------- + * + * 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?
Yeah. Array access based on an procnum index is still feeling like
the natural thing to do here (I think I've mentioned that upthread?).
It's cheap to initialize, cheaper to access for reads and writes of
the data than one or two hash tables. For hot paths, that would
matter.
--
Michael
Hi,
On Mon, Sep 21, 2026 at 05:17:04AM -0400, Andres Freund wrote:
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.
Thanks for looking at it!
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...
Yeah, the idea is not to repeat these tests for each statistics kind. I had in
mind to use WAL as a representative kind to test the consistency behavior.
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?
A background worker can access shared memory without establishing a database
connection, and therefore without calling InitPostgres(). The intent is to verify
that such a worker gets its per-backend entries during initialization and can
publish its statistics through the nonblocking path while remaining alive.
The nonblocking path is important because it cannot create a missing per-backend entry:
it succeeds only if pgstat_initialize() has already created and cached it. A forced
report could create the entry on demand and hide exactly an initialization bug
this test is intended to catch.
The checkpointer and worker_spi use forced reports, while logical replication workers
establish a database connection and can later force a report. Thus, none covers
this case.
Do you think this justifies the additional test code?
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.
Yeah, will do in a next version, thanks.
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.
The test needs a process to start after the statistics snapshot has been built.
The isolation tester creates all its sessions before running any steps, so it
cannot cover this case.
I don't see an existing TAP test where this would fit naturally, and adding it to
an unrelated one only to reuse its node does not seem great either.
I think the coverage is useful because it verifies that a snapshot does not
fabricate entries for processes that did not exist when it was built.
Do you think this justifies the dedicated cluster, or would you prefer dropping
this test?
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.
Yeah, each live process is expected to have an entry for these kinds.
The distinction though is between live processes and MaxBackends. A static array
has to be sized for every possible ProcNumber at startup, even if most slots are
never used.
If nearly all MaxBackends slots are live, then I agree that an array would likely
be preferable. The case I was trying to avoid is MaxBackends being substantially
higher than the usual number of live processes.
An earlier POC used such arrays. With max_connections=10000, it reserved about
33.5MB for the three kinds regardless of the number of live processes.
Global queries also had to scan unused slots or would require another structure
to track active ones.
The dshash allocates entries only for processes that actually exist. Also, each
process caches its entries during initialization, so the flush path does not
perform a hash lookup.
Does that explanation make sense, or do you still think a fixed array would be
preferable?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi,
On Tue, Sep 22, 2026 at 09:16:30AM +0900, Michael Paquier wrote:
On Mon, Sep 21, 2026 at 05:17:04AM -0400, Andres Freund wrote:
On 2026-09-21 07:15:44 +0000, Bertrand Drouvot wrote:
+/* ------------------------------------------------------------------------- + * + * 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?Yeah. Array access based on an procnum index is still feeling like
the natural thing to do here (I think I've mentioned that upthread?).
Yeah you did mention it. I took your reply in [1]/messages/by-id/anlVx_TD_Q8He5uf@paquier.xyz as agreeing with the concern
about reserving memory for mostly unused slots, though not necessarily with using
a dshash. Is your preference still for the array despite that memory cost?
[1]: /messages/by-id/anlVx_TD_Q8He5uf@paquier.xyz
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com