Re: Table synchronization cannot resume after a subscriber crash

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