Re: Adding locks statistics

Hi,

On Thu, Feb 19, 2026 at 01:06:52PM +0900, Michael Paquier wrote:
> On Tue, Feb 17, 2026 at 04:33:54PM +0000, Bertrand Drouvot wrote:
> > Okay, done that way in the attached. To avoid overhead due to timing as much as
> > possible, the patch simply relies on log_lock_waits and deadlock_timeout. It means
> > that it relies on the existing code, and increments waits and wait_time only if
> > log_lock_waits is on and if the session waited longer than deadlock_timeout.
> > 
> > I did not want to dissociate the waits and wait_time increments so that their
> > ratio could still make sense.
> > 
> > That sounds like a good compromise, thoughts?
> 
>              else if (myWaitStatus == PROC_WAIT_STATUS_OK)
> +            {
> +                /* Increment the lock statistics counters */
> +                pgstat_count_lock_waits(locallock->tag.lock.locktag_type);
> +                pgstat_count_lock_wait_time(locallock->tag.lock.locktag_type, msecs);
> 
> Not sure that it makes much sense to me to rely on log_lock_waits
> being enabled to decide if this count and this time are aggregated.
> The log information and the stats gathering are two separate things.
> Wouldn't it make more sense to call pgstat_count_lock_waits() outside
> of this code path, when we know myWaitStatus?

> While relying on the time calculating for the logs data is a good
> idea, it seems to me that we should have a separate GUC to enable this
> number, like a new track_lock_timings?  If track_lock_timings or
> log_lock_waits is enabled, we should calculate the time difference.
> All these decisions also depends on what deadlock_state holds on top
> of myWaitStatus, I guess..

The idea was to avoid adding a new GUC and I did not want to increment the
waits independently of the wait time (so that wait time/waits could make
sense).

That said, your point of view also makes (more) sense, so in the attached:

- adds a new GUC (namely track_lock_timing)
- tracks the wait_time if the GUC is on and the session waited longer than
deadlock_timeout
- when wait_time is incremented, then a new timed_waits counter is also
incremented (so that wait_time / timed_waits makes sense)
- waits is incremented unconditionally

Note that due to the new GUC behavior (wait_time incremented only if we waited
longer than deadlock_timeout), then it is on by default (same idea as for
2aac62be8cb).

Regards,

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