Re: Table synchronization cannot resume after a subscriber crash

Hi Zhijie,

Thanks for the review! I incorporated your feedback and rebased the changes
on top of HEAD. The updated patches are attached in `git format-patch`
format.

The overall approach is the same in spirit, with a simpler implementation
based on your suggestions. I also expanded the comment to explain why the
apply worker waits for origin cleanup before marking the table READY.

The test covers crashes both before and after the slot drop. The after-drop
case fails with the old code, while both cases pass with the fix.

Let me know how this iteration looks!

Best

On Sun, 8 Nov 2026 at 06:06, Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:

> Hi,
>
> On Sat, Nov 7, 2026 at 12:00 AM Rafael Chacón wrote:
> > I'm an engineer working on Multigres at Supabase. While debugging flaky
> > tests in Multigres, we found what we believe is a bug in PostgreSQL
> > logical replication.
> >
> > I reproduced this on PostgreSQL 17.11, built from source on macOS arm64.
> >
> > After a subscriber restart, a table remains stuck in |FINISHEDCOPY| (|
> > pg_subscription_rel.srsubstate = 'f'|), while already synchronized
> > tables continue receiving changes.
> >
> > The table sync worker repeatedly reports:
> >
> > |ERROR: could not start WAL streaming: ERROR: replication slot
> > "pg_16405_sync_16398_7704351398798576014" does not exist|
> >
> ...
> >
> > I put together a draft patch that commits and flushes the |SYNCDONE|
> > state before deleting the slot. It also allows a restarted worker to
> > complete the cleanup if interrupted between those steps.
> >
> > I'm attaching the patch and a test using injection points to reproduce
> > the issue.
> >
> > Does this understanding look correct, or is there a recovery path I'm
> > overlooking? If this is indeed a bug, I'd appreciate some guidance on
> > the proposed approach. Happy to refine the patch for |REL_17_STABLE|.
>
> Thanks for the report and the patches.
>
> Could you please rebase the patches onto HEAD and format them using git
> format-patch with a commit message ? Since your patch not only changes
> when the tablesync slot is dropped, but also moves the replication
> origin drop logic to the tablesync worker, it would be good to explain
> that change and the reason for it in the commit message.
>
> A similar idea was previously discussed in another thread [1] (regarding
> replication origin drop). In that thread, we also considered letting a
> restarted tablesync worker drop the slot and origin again if the
> previous worker failed. We did not push that patch at the time because
> the failure window seemed very narrow with no user reports, and that
> patch uses a different approach by adding a new catalog relstate
> (PRE_SYNCDONE which acts the same as the old SYNCDONE state, and the
> real SYNCDONE means slot and origins are cleaned up), which felt
> invasive for that case alone.
> However, since you encountered this issue in practice and your approach
> avoids adding a new relstate, I think it is worth exploring your
> approach and comparing it with the previous one.
>
> Here are a few comments on 03-durable-handoff-fix.patch:
>
> 1.
>
> +/*
> + * REFRESH may have removed or re-added this table while our cached
> + * state was stale. The locks keep it from changing again.
> + */
> +if (GetSubscriptionRelState(MyLogicalRepWorker->subid,
> + rstate->relid,
> + &rstate->lsn) !=
> ...
> +{
> + table_states_validity = SYNC_TABLE_STATE_NEEDS_REBUILD;
> + continue;
> +}
>
> I think this check is not needed. IIUC, the race between REFRESH
> PUBLICATION and the apply worker marking a table READY already exists on
> HEAD, so it is not directly related to the issue this patch is trying to
> solve. Or is there a specific issue introduced by this patch that
> requires this check?
>
>
> 2.
>
> +/*
> + * Lock the subscription and relation states in the same order as
> + * we are doing during DDL commands to avoid deadlocks. See
> + * AlterSubscription_refresh.
> + */
> +LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid,
> + 0, AccessShareLock);
> +if (!rel)
> + rel = table_open(SubscriptionRelRelationId, RowExclusiveLock);
>
> This locking was originally added because the apply worker dropped the
> replication origin here before calling UpdateSubscriptionRelState().
> Since your patch removes the origin drop from the apply worker, I think
> this explicit locking can be removed as well (letting
> UpdateSubscriptionRelState() acquire the locks when updating the state
> to READY).
>
> 3.
>
> -rstate->state = syncworker->relstate;
> -rstate->lsn = syncworker->relstate_lsn;
> +if (rstate->state != SUBREL_STATE_SYNCDONE &&
> + syncworker->relstate != SUBREL_STATE_SYNCDONE)
> +{
> + rstate->state = syncworker->relstate;
> + rstate->lsn = syncworker->relstate_lsn;
> +}
> if (rstate->state == SUBREL_STATE_SYNCWAIT)
>
> Instead of adding this check to avoid overwriting rstate->state, I think
> we can simply save syncworker->relstate in a local variable and use it
> for the subsequent SUBREL_STATE_SYNCWAIT checks without modifying
> rstate->state and rstate->lsn.
>
> [1]
>
> https://www.postgresql.org/message-id/OS3PR01MB5718B099C8DA6F91FDA26EEF94D79@OS3PR01MB5718.jpnprd01.prod.outlook.com
>
>
> --
> Best Regards,
> Zhijie Hou
>