On 2023-10-04 03:00, James Coleman wrote:
> and I think
> what we need to do is explicitly disallow running this code any time
> we are inside of lock acquisition code.
Updated patch to check if any locks have already been acquired by
examining MyProc->heldLocks.
I'm not sure this change can "disallow running this code `any time` we
are inside of lock acquisition code", but as far as select1.trace, which
you shared, I believe it can prevent running explain codes since it must
have set MyProc->heldLocks in LockAcquireExtended() before WaitOnLock():
```
/*
* Set bitmask of locks this process already holds on this
object.
*/
MyProc->heldLocks = proclock->holdMask;
..(snip)..
WaitOnLock(locallock, owner);
```
On 2023-10-07 00:58, Andres Freund wrote:
> How so? We shouldn't commonly acquire relevant locks while executing a
> query?
> With a few exceptions, they should instead be acquired t the start of
> query
> processing. We do acquire a lot of lwlocks, obviously, but we don't
> process
> interrupts during the acquisition / holding of lwlocks.
>
> And presumably the interrupt would just be processed the next time
> interrupt
> processing is happening?
Thanks for your comments!
I tested v30 patch with
v28-0002-Testing-attempt-logging-plan-on-ever-CFI-call.patch which makes
CFI() call ProcessLogQueryPlanInterrupt() internally, and confirmed that
very few logging queries failed with v30 patch.
This is a result in line with your prediction.
```
$ rg -c'ignored request for logging query plan due to lock confilcts'
postmaster.log
8
```
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
