Re: Adding locks statistics
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Greg Sabino Mullane <htamfids@gmail.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-08-11T23:44:58Z
Lists: pgsql-hackers
On Mon, Aug 11, 2025 at 02:53:58PM -0700, Jeff Davis wrote: > On Mon, 2025-08-11 at 13:54 -0400, Greg Sabino Mullane wrote: > > Great idea. +1. Here is a quick overall review to get things started. > > Can you describe your use case? I'd like to understand whether this is > useful for users, hackers, or both. This is a DBA feature, so the questions I'd ask myself are basically: - Is there any decision-making where these numbers would help? These decisions would shape in tweaking the configuration of the server or the application to as we move from a "bad" number trend to a "good" number trend. - What would be good numbers? In this case, most likely a threshold reached over a certain period of time. - Would these new stats overlap with similar statistics gathered in the system, creating duplication and bloat in the pgstats for no real gain? Looking at the patch, the data gathered is this, and I don't think that we have metrics gathered in the system to get an idea of the contention in this area, for timeouts and deadlocks: + PgStat_Counter requests; + PgStat_Counter waits; + PgStat_Counter timeouts; + PgStat_Counter deadlock_timeouts; + PgStat_Counter deadlocks; + PgStat_Counter fastpath; Isn't the "deadlock_timeout" one an overlap between timeout and deadlock? Having three counters to track two concepts seems strange to me. Regarding the fastpath locking, you'd want to optimize the fastpath to be close to the number of requests. If you had these numbers, one thing that I could see a DBA do is tune max_locks_per_transaction, which is what influences the per-backend limit of fast-path locks, with FastPathLockGroupsPerBackend. Using static counters for the pending data is going to be necessary when these are called in critical sections, which is I guess why you've done it this way, right? -- Michael
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix injection point detach timing problem in TAP test for lock stats
- 3284e3f63cf0 19 (unreleased) landed
-
Use single LWLock for lock statistics in pgstats
- 404a17c155ac 19 (unreleased) landed
-
Add tests for lock statistics, take two
- 557a9f1e3e62 19 (unreleased) landed
-
Remove isolation test lock-stats
- 7c64d56fd976 19 (unreleased) landed
-
Avoid including clog.h in proc.h
- f227b7b20c36 19 (unreleased) landed
-
Don't include storage/lock.h in so many headers
- 2102ebb1953f 19 (unreleased) landed
-
Add support for lock statistics in pgstats
- 4019f725f5d4 19 (unreleased) landed
-
Move some code blocks in lock.c and proc.c
- a90d86518243 19 (unreleased) landed
-
Move declarations related to locktags from lock.h to new locktag.h
- 322bab79744d 19 (unreleased) landed