On Wed, Mar 18, 2026 at 02:51:01PM +0000, Bertrand Drouvot wrote:
> This is done that way in the attached, so that we don't need the extra output 
> in
> the _1.out file and the test time is reduced (since the deadlock timeout is 
> set
> to 10ms in the test, I changed the sleep time to 50ms (I did not want to be 
> very
> close to 10ms)).

> +        <structfield>waits</structfield> <type>bigint</type>
> +       </para>
> +       <para>
> +        Number of times a lock of this type had to wait because of a
> +        conflicting lock. Only incremented when <xref 
> linkend="guc-log-lock-waits"/>
> +        is enabled and the lock was successfully acquired after waiting 
> longer
> +        than <xref linkend="guc-deadlock-timeout"/>.
> +       </para>
> +      </entry>

It does not make much sense to me to decide that the counter is
incremented if a GUC related to a control of the logs generated is
enabled.  It's a fact that log_lock_waits is enabled by default these
days, hence we will be able to get the time calculation for free for
most deployments, but it seems inconsistent to me to not count this
information if the GUC is disabled.  We should increment the counter
and aggregate the time spent on the wait all the time, no?

+ * Copyright (c) 2021-2025, PostgreSQL Global Development Group 

Incorrect date at the top of pgstat_lock.c.

storage/lock.h is included in pgstat.h as LOCKTAG_LAST_TYPE is wanted
for the new lock stats structure.  That would pull in a lot of header
data into pgstat.h.  How about creating a new header that splits a
portion of lock.h into a new file?  LockTagType, LOCKTAG_LAST_TYPE,
LockTagTypeNames at least and perhaps a few other things?  Or we could
just limit ourselves to a locktag.h with these three, then include the
new locktag.h in pgstat.h.

It would be nice to document at the top of the new spec file what this
test is here for, and perhaps name it stats-lock instead?
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to