Re: Add “FOR UPDATE NOWAIT” lock details to the log.

On 2024/12/19 17:21, Yuki Seino wrote:
>> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=3c0fd64fec8ed6fa3987c33f076fcffbc3f268c3
> Rebased and added new GUC log_lock_failure and minor fixes. Currently only NOWAIT errors are supported.

Thanks for updating the patch!

+        Currently, only lock failures due to <literal>NOWAIT</literal> are
+        supported.  The default is <literal>off</literal>.  Only superusers

This GUC also affects SKIP LOCKED, so that should be documented.

Regarding the GUC name log_lock_failure, I'm not sure it's the best fit,
but I don't have a better suggestion at the moment. I'll keep considering alternatives.

If CollectLockHoldersAndWaiters() is used only in proc.c,
it should be made a static function.

+/*
+ * we loop over the lock's procLocks to gather a list of all
+ * holders and waiters. Thus we will be able to provide more
+ * detailed information for lock debugging purposes.

For the source comment of CollectLockHoldersAndWaiters(),
could you follow the style of other functions in proc.c?

Why is the logic to report lock holders and waiters implemented within JoinWaitQueue()?
Wouldn't it be better placed right after JoinWaitQueue() is called in
LockAcquireExtended(), similar to DeadLockReport()? One issue with
the current implementation is that partitionLock is held in exclusive mode
while collecting and reporting lock holders and waiters. If the logic is moved
after JoinWaitQueue() in LockAcquireExtended(), lock holders and waiters could be
processed after releasing partitionLock. Note, however, that partitionLock might
still need to be taken again in shared mode during the collection.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION