Re: Add CHECK_FOR_INTERRUPTS in polling loop code path in XactLockTableWait

Hi Fujii,
Thanks a lot for reviewing!

> > This update amends the previous patch by replacing the global
> > XactLockTableWait_us with a local wait_us variable inside
> > XactLockTableWait. Now each call resets its backoff to 1 ms, doubling
> > up to 1 s when oper == XLTW_None. This eliminates shared‐state bugs
> > (where multiple backends or successive calls would otherwise inherit a
> > stale, high sleep value).
> >
> > I remain concerned about relying on oper == XLTW_None to signal
> > “logical‐replication” use cases, since:
> > It couples backoff behavior to the absence of an error‐callback
> > context. Future callers passing oper == None for other reasons would
> > inadvertently get progressive backoff.
> > The intent (“I need a long‐wait backoff”) isn’t self‐evident from oper.
>
> I think that this concern is right and we should avoid this approach.

+1

>
> > An alternative is to add a separate boolean or enum argument (e.g.
> > bool isLogicalReplication) so that callers explicitly opt in to
> > backoff. That approach is more verbose but avoids hidden coupling and
> > is easier to maintain if additional contexts require different wait
> > policies. If ConditionalXactLockTableWait ever needs backoff, it would
> > benefit from the same explicit flag.
>
> I prefer this approach over "oper == XLTW_None" one.

Agreed. My worry was that introducing a new argument adds many
call-site changes, and XLTW_Oper is primarily for error-context
callbacks.

>
> Just idea, if XactLockTableWait() is expected to finish within a few seconds
> after acquiring the lock, how about this approach: keep sleeping for 1ms
> until the total sleep time reaches 10s (10s is just an example),
> and after that, start doubling the sleep duration each cycle, up to
> a maximum of 10s. That is, even in non-"create replication slot" case,
> if the total sleep time exceeds 10s, it seems safe to double the sleep time
> up to 10s. This way, we stay responsive early on but can back off more
> aggressively if needed. Thought? Anyway we probably need to study
> XactLockTableWait() behavior more closely.
>

I agree—we need to study XactLockTableWait() usage more closely.
Picking a fixed threshold could be tricky across different workloads.
Exposing it as a GUC for this one also feels heavy.

> Thanks for the patch! I haven't reviewed it yet, but since this is
> a v19 item, please add it to the next CommitFest so we don't lose
> track of it.
>
> Also, I think it would be better to split the addition of the wait event
> and the introduction of exponential backoff in XactLockTableWait() into
> separate patches. They serve different purposes and can be committed
> independently.

Understood—I’ll split them and move both to July’s CommitFest.

>
> BTW, regarding the discussion on the list, please avoid top-posting;
> bottom-posting is the preferred style on this mailing list.
>

Good point—I’ll use bottom-posting going forward.