Re: Redesign per-backend statistics

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