Re: Add “FOR UPDATE NOWAIT” lock details to the log. - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: Add “FOR UPDATE NOWAIT” lock details to the log.
Date
Msg-id 8f754553-9b0b-4a83-bd3a-b7dea35501b1@oss.nttdata.com
Whole thread Raw
In response to Re: Add “FOR UPDATE NOWAIT” lock details to the log.  (Yuki Seino <seinoyu@oss.nttdata.com>)
List pgsql-hackers

On 2025/02/20 15:27, Yuki Seino wrote:
> 
> On 2025/02/19 1:08, Fujii Masao wrote:
>> Just an idea, but how about updating ConditionalXactLockTableWait() to do the followings?
>> - Use LockAcquireExtended() instead of LockAcquire() to retrieve the LOCALLOCK value.
>> - Generate a log message about the lock holders, from the retrieved the LOCALLOCK value.
>> - Return the generated log message to the caller (heap_lock_tuple()), allowing it to log the message.
> Thank you for your suggestion. I have two issues to discuss:
> 
> 
> ### 1. Issue with LockAcquireExtended()
> 
> It appears that in the dontWait case, LockAcquireExtended() is removing the local lock (locallock).
> 
> ```
> if (locallock->nLocks == 0)
>      RemoveLocalLock(locallock);
> ```
> 
> Instead, the removal of locallock should be handled by ConditionalXactLockTableWait().

RemoveLocalLock() doesn't seem to remove the necessary LocalLock information
for generating the lock failure log message. Is that correct?

One idea I considered is using the LocalLock returned by LockAcquireExtended(),
but this might complicate the code more than expected. A simpler approach could
be adding a new argument, like logLockFailure, to LockAcquireExtended(),
allowing it to log the failure message when set to true. This change would
affect LockAcquireExtended() and some ConditionalLockXXX() functions,
but it doesn't seem too invasive.

As for LockAcquire(), I don't think it needs a new argument. If any
ConditionalLockXXX() functions call LockAcquire(), we could modify them to
call LockAcquireExtended() instead, enabling logging when needed.


> ### 2. Issue with the LOCK TABLE Case
> 
> For the LOCK TABLE scenario, RangeVarGetRelidExtended() calls ConditionalLockRelationOid(), which seems to require a
similarmodification.
 

Yes, if we want to support LOCK TABLE NOWAIT and other NOWAIT cases,
these additional updates would be required. However, I suggest focusing
on row-level locks with SELECT ... NOWAIT first, then expanding to
other cases later.

Regards,

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




pgsql-hackers by date:

Previous
From: John Naylor
Date:
Subject: Re: Improve CRC32C performance on SSE4.2
Next
From: Ajin Cherian
Date:
Subject: Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding