Re: Adding locks statistics

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>, Jeff Davis <pgsql@j-davis.com>, Greg Sabino Mullane <htamfids@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2026-02-16T10:10:21Z
Lists: pgsql-hackers
Hi,

On Fri, Feb 13, 2026 at 04:13:39PM -0500, Andres Freund wrote:
> Hi,
> 
> On 2026-02-13 10:24:52 +0000, Bertrand Drouvot wrote:
> > 
> > That's fine by me. We could still add the others in the future if we feel the
> > need. Done that way in the attached.
> 
> I'm not sure that it's unproblematic to add multiple pgstat count calls to
> every lock acquisition, particularly if it's a fastpath acquisition or a
> virtualxid lock. Notably these are external function calls, not just
> increments of a counter in an inline function.

Yeah, we could create inline functions instead.

> I also don't really know what one would do with some of the information?
> 
> What does the number of virtualxid lock acquisitions tell you that the numbers
> of transactions doesn't already tell you in a more understandable way?

I agree not that much, except being able to compute a transactions/virtualxid
ratio or such. Also the idea was to report the same as pg_locks so that one
could start sampling pg_locks if he needs more details.

> What does it tell you that the deadlock checker ran N times? It notably
> doesn't count deadlocks, it counts how often we checked for deadlocks.
> 
> The percentage of fastpath locks also seems not really informative, because
> that could be because we ran out of space for fastpath locks, or because a
> lock mode that's ineligible for fastpath locks was used.

Right, maybe we could add an "exceed fastpath slots" or such counter?

> What I would actually count is the amount of time waiting for locks, that
> seems vastly more useful than the number of acquisitions.  We already do a
> GetCurrentTimestamp() inside the timer activations for deadlock timeout, we
> probably can figure out a way to reuse that to reduce the increase in overhead
> due to timing.  We could also just count the wait time after the deadlock
> check has run.

Yeah, providing the wait_time would be great. Just to be sure, are you suggesting
to remove all the fields (i.e requests, timeouts, deadlock_timeouts and fastpath)
and just add a wait_time field instead? I think that keeping requests would make
sense to be able to get the average wait time per request.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix injection point detach timing problem in TAP test for lock stats

  2. Use single LWLock for lock statistics in pgstats

  3. Add tests for lock statistics, take two

  4. Remove isolation test lock-stats

  5. Avoid including clog.h in proc.h

  6. Don't include storage/lock.h in so many headers

  7. Add support for lock statistics in pgstats

  8. Move some code blocks in lock.c and proc.c

  9. Move declarations related to locktags from lock.h to new locktag.h