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
