On 2025/02/21 10:28, Fujii Masao wrote:
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 similar
modification.
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.
Thanks for the advice.
I have taken your advice and am working on a patch.
But, I have a problem.
There are 4 locations where LockWaitError is determined.
3 of them could be reproduced, but 1 could not.
I will check more in depth.
(1)
https://github.com/postgres/postgres/blob/master/src/backend/access/heap/heapam.c#L4946
tx1=# begin;
tx1=# select * from pgbench_accounts where aid = '1' for update;
tx2=# select * from pgbench_accounts where aid = '1' for update nowait;
(2)
https://github.com/postgres/postgres/blob/master/src/backend/access/heap/heapam.c#L4905
tx1=# begin;
tx1=# select * from pgbench_accounts where aid = '1' for share;
tx2=# begin;
tx2=# select * from pgbench_accounts where aid = '1' for share;
tx3=# select * from pgbench_accounts where aid = '1' for update nowait;
(3)
https://github.com/postgres/postgres/blob/master/src/backend/access/heap/heapam.c#L5211
tx1=# begin;
tx1=# select * from pgbench_accounts where aid = '1' for update;
tx2=# begin;
tx2=# select * from pgbench_accounts where aid = '1' for update;
tx3=# select * from pgbench_accounts where aid = '1' for update nowait;
(4)
https://github.com/postgres/postgres/blob/master/src/backend/access/heap/heapam_handler.c#L463
...I don't know how to reproduce it.
Regards,
