Re: sequencesync worker race with REFRESH SEQUENCES

On Tue, 22 Sept 2026 at 19:21, Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
> >
> > Attached is v2, which takes a slightly different approach to the same
> > problem. Instead of stopping the sequence sync worker when a refresh
> > removes a sequence, copy_sequence() now checks whether the sequence is
> > still part of the subscription before updating it and skips it if it
> > is no longer subscribed.
>
> This approach makes sense to me.
>
> I didn't find any major issues in the patch, but I have a few questions:
>
> 1.
>
> + /*
> + * The sequence may no longer be part of the subscription. There is
> + * nothing left to synchronize, so leave the local sequence alone and let
> + * the caller skip it.
> + */
> + if (GetSubscriptionRelState(MySubscription->oid, seqoid,
> + &statelsn) == SUBREL_STATE_UNKNOWN)
>
> Can we simply use SearchSysCacheExists2 to check for the subrel entry here ?

Yes, it can be used.

> 2.
>
> + rel = table_open(SubscriptionRelRelationId, RowExclusiveLock);
> ...
> + table_close(rel, NoLock);
> + table_close(rel, NoLock);
> + table_close(rel, NoLock);
>
> The patch adds 3 table_close calls in each return branch. Would it be possible
> to delay the table_open to just before UpdateSubscriptionRelState, so that only
> one close call is needed?

Modified

The attached v3 version patch has the changes for the same.

Regards,
Vignesh