Re: sequencesync worker race with REFRESH SEQUENCES

On Sat, Jul 18, 2026 at 9:05 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Jul 17, 2026 at 5:23 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > Thinking some more on this idea, we can achieve to close the race
> > condition Noah reported without giving ERROR. The idea is that REFRESH
> > SEQUENCES can acquired lock on pg_subscription_rel in
> > AccessExclusiveLock to prevent a race with the apply worker
> > re-launching a sequence sync worker while we reset the states. Even if
> > a new worker starts, it can't progress: it opens pg_subscription_rel
> > in AccessShareLock mode in LogicalRepSyncSequences() and will block
> > until we commit, by which time the sequences are INIT and it will sync
> > the latest values. After acquiring, REFRESH SEQUENCES command can stop
> > the sequencesync worker and then made all sequence states to INIT. The
> > stop-and-restart design along with lock ensures that after REFRESH
> > SEQUENCES is finished all sequences will be guaranteed to have new
> > values. Attached patch implements this idea. I did basic testing and
> > it seems to be working but needs more review and test. I also want to
> > once review the decision of locking pg_subscription_rel.
> >
> > Note: I'll think some more on the above idea and if I or someone
> > didn't find any hole in it then will revert f38afa4ab tomorrow and
> > then make this patch ready early next week.
> >
>
> I have pushed the revert. Attached patch fixes the race without taking
> lock on pg_subscription_rel. This should be race-free because
> AlterSubscription() already holds AccessExclusiveLock on the
> subscription object. That lock blocks a running worker's
> UpdateSubscriptionRelState(), which takes AccessShareLock on the
> object, and also any worker the apply worker re-launches, because a
> new worker takes AccessShareLock on the object in
> InitializeLogRepWorker() before it reads pg_subscription_rel. Such a
> worker cannot act on the sequence states until the refresh commits, by
> which time they are reset to INIT and it will synchronize the latest
> publisher values.
>

The patch looks good. These are my obseravtions while testing:

1. If REFERSH-seq is triggered before the current seq-worker has not
reached its first UpdateSubscriptionRelState(), the seq-worker is
immediately killed and REFRESH proceeds.
2. If REFERSH-seq is triggered while a running seq-worker has already
processed one or few UpdateSubscriptionRelState(), REFRESH waits for
the worker to commit that batch, then the worker is killed and REFRESH
proceeds.
3. If seq-worker is re-launched while REFERSH-seq is changing states,
seq-worker is blocked to acquire AccessShareLock on
SubscriptionRelationId in InitializeLogRepWorker. Worker proceeds only
when REFERSH commits.

thanks
Shveta