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: https://www.postgresql.org/message-id/CAA5RZ0supQBxSkh=CWB39=j+cl3hhclpki3tcbk0b1r4fes...@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
signature.asc
Description: PGP signature
