Hi, On Tue, Sep 22, 2026 at 5:34 PM vignesh C <[email protected]> wrote: > > On Tue, 22 Sept 2026 at 08:42, Nikolay Samokhvalov <[email protected]> wrote: > > The attached patch stops the sequencesync worker in the removal loop, > > as the tablesync loop and AlterSubscription_refresh_seq() do, with the > > same lock argument. It adds a test to 036_sequences.pl using the > > publisher-side blocking trick that file already uses. The test fails on > > unpatched REL_19_STABLE with the error above and the subscription > > disabled, and passes with the fix. > > Thanks Nik for reporting this. > > 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 ? 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? Best Regards, Zhijie Hou
