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

Attachment: signature.asc
Description: PGP signature

Reply via email to