Re: sequencesync worker race with REFRESH SEQUENCES

On Tue, 22 Sept 2026 at 08:42, Nikolay Samokhvalov <nik(at)postgres(dot)ai> wrote:
>
> On Fri, Jul 31, 2026, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > Here's the summary of the findings:
> > Finding 1: Refresh paths race the in-flight sequencesync worker -
> > committed (45cf7b1e5bf923ca48dfd9aa5001bdd0630d11c3)
>
> The PG19 open-items page still lists this item under "resolved before
> 19beta3". I left that entry unchanged; please decide whether it should
> be reopened.

I will open an item after a couple of days, once we agree that this
needs to be fixed, unless you think otherwise.

> Part (B) of that finding is still there on REL_19_STABLE (b73d13c3)
> and master (d39fda1c). 45cf7b1e5bf stops the worker in
> AlterSubscription_refresh_seq(), but the sequence-removal loop in
> AlterSubscription_refresh() still only takes AccessExclusiveLock on
> pg_subscription_rel and calls RemoveSubscriptionRel(). Unlike the table
> loop right above it, it does not stop the sequencesync worker. A worker
> that already has the sequence in its INIT list fails once the refresh
> commits:
>
> ERROR: subscription relation 16390 in subscription 16392 does not exist
>
> The batch is rolled back and sync_seq_error_count goes up. With
> disable_on_error = true the whole subscription is disabled, tables
> included. The worker has also already applied the publisher value to the
> local sequence, which is no longer subscribed.
>
> Deterministic reproducer on REL_19_STABLE, no injection points:
>
> publisher:
> create table t (id int primary key);
> create sequence s1; create sequence s2;
> create publication pub_seq for all sequences;
> create publication pub_tab for table t;
>
> subscriber:
> create table t (id int primary key);
> create sequence s1; create sequence s2;
> create subscription sub1 connection '...' publication pub_seq
> with (disable_on_error = true, enabled = false);
>
> publisher, session A, keep it open:
> begin; drop sequence s1;
>
> subscriber:
> alter subscription sub1 enable;
> -- wait until the sequencesync worker's batch query is blocked on
> -- the publisher: pg_locks shows a not-granted AccessShareLock on s1
> alter subscription sub1 set publication pub_tab;
>
> publisher, session A:
> rollback;
>
> subscriber, once the worker has exited:
> select subenabled from pg_subscription; -- f
> select sync_seq_error_count from pg_stat_subscription_stats; -- 1
>
> 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.

I preferred the v2 approach for the following reasons: a) It avoids
discarding progress for other sequences in the batch. Sequences are
synchronized in batches of up to MAX_SEQUENCES_SYNC_PER_BATCH (100) in
a single transaction. Stopping the worker because one sequence was
removed discards the catalog progress for up to 99 other sequences.
Those sequences return to INIT and have to be fetched and synchronized
again by the next worker. b) The cost of stopping the worker depends
on the batch size. If the batch size is increased in the future, v1
would silently make this case more expensive. c) It can stop the
worker unnecessarily. AlterSubscription_refresh() removes the
pg_subscription_rel row regardless of its current state. Thus, even
removing a sequence that was already marked READY can stop the worker
while it is processing an unrelated set of sequences.

With v2, the check is made at the point where the sequence is about to
be updated, so a sequence that is no longer part of the subscription
is simply skipped.

A test for this is possible using the same publisher-side uncommitted
DROP as in the test above, but I don't think it is necessary. The
scenario requires two subscription DDLs to overlap with an ongoing
sequence synchronization, which is rare in practice, and the result is
now just a skipped sequence and a log message. Adding such a test
would require another background session, two polling loops, and a
full re-synchronization on every run of 036_sequences.pl. I don't
think the extra test is necessary for such a rare case.

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

Regards,
Vignesh