Date: Fri, 7 Aug 2026 12:31:47 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org, Andres Freund <andres@anarazel.de>,
	Sami Imseih <samimseih@gmail.com>
Subject: Re: Redesign per-backend statistics
Message-ID: <anXQM2PoJ+6iYYT2@bdtpg>
References: <anCjoFkGah/uB7lw@bdtpg>
 <anU3OE2403PZdwJL@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <anU3OE2403PZdwJL@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/anXQM2PoJ%2B6iYYT2%40bdtpg>
Precedence: bulk

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:
> https://www.postgresql.org/message-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