Hi, On Tue, Nov10, 2026 at 04:04 AM Masahiko Sawada wrote:
Hi all,While reviewing slotsync.c, I hit the assertion failure: TRAP: failed Assert("MyReplicationSlot->data.persistency == RS_TEMPORARY"), slotsync.c:364 IIUC this is a regression of commit 788ec96d591 (CC'ed Hou and Amit). In update_local_synced_slot(), we try to advance the local slot only when the remote slot is ahead of it, and if the standby cannot reach a consistent snapshot we set the skip reason to SS_SKIP_NO_CONSISTENT_SNAPSHOT so that update_and_persist_local_synced_slot() doesn't persist the slot. However, LogicalSlotAdvanceAndCheckSnapState() has already advanced confirmed_flush_lsn to the remote's value in that case (restart_lsn and catalog_xmin were already equal to the remote's when the slot was created). So in the next cycle the remote slot is no longer ahead, we skip the advance entirely, and update_slotsync_skip_stats() overwrites the skip reason with SS_SKIP_NONE. update_and_persist_local_synced_slot() then persists the slot and reports it as sync-ready even though nothing has changed since the previous cycle. Once the remote slot advances again and the standby still cannot find a consistent snapshot, the assertion above fails. On non-assert builds, IIUC the problem is worse: the persisted slot can be used after promotion, and decoding from it skips the transactions between its restart_lsn and the first consistent point, causing data loss. Before that commit, update_and_persist_local_synced_slot() also checked found_consistent_snapshot returned by update_local_synced_slot() for the current cycle, so a cycle that didn't attempt to advance the slot never persisted it. I've attached a TAP test to reproduce this issue, generated by AI.
Thanks for reporting the issue.I agree with your analysis, and I think we can fix it by preserving SS_SKIP_NO_CONSISTENT_SNAPSHOT in update_local_synced_slot() when the remote slot has not advanced since a previous cycle failed to build a consistent snapshot.
Attached is a patch for the fix. I also tried to simplify and stabilize the test and merged it into 040_standby_failover_slots_sync.pl in case we want to include a test.
-- Best Regards, Zhijie Hou
From 337c59105081e2b7b37183b06330c5d15828ed9b Mon Sep 17 00:00:00 2001 From: Zhijie Hou <[email protected]> Date: Tue, 10 Nov 2026 17:06:11 +0800 Subject: [PATCH v1] Fix premature persistence of synced slots without a consistent snapshot. During slot synchronization, update_local_synced_slot() attempts to advance the local slot only when the remote slot's LSN or catalog_xmin is ahead of the local slot. If the standby cannot build a consistent snapshot from the slot's restart_lsn, the skip reason is set to SS_SKIP_NO_CONSISTENT_SNAPSHOT so that update_and_persist_local_synced_slot() keeps the slot temporary. However, LogicalSlotAdvanceAndCheckSnapState() has already updated the local slot's confirmed_flush to the remote slot's value in that cycle. In the next synchronization cycle, if the remote slot has not advanced, update_local_synced_slot() skips the advance check and resets the skip reason to SS_SKIP_NONE. Following commit 788ec96d591, update_and_persist_local_synced_slot() relies on slotsync_skip_reason to decide whether to persist the slot, causing it to prematurely persist the slot and mark it sync-ready before a consistent snapshot is built. Fix this by preserving SS_SKIP_NO_CONSISTENT_SNAPSHOT in update_local_synced_slot() when the remote slot has not advanced since a previous cycle failed to build a consistent snapshot. Reported-by: Masahiko Sawada <[email protected]> Author: Zhijie Hou <[email protected]> Discussion: https://postgr.es/m/cad21aoddn9qj2q5dnkw9daazvoh-9yef7jw64qphjgqi3yj...@mail.gmail.com Backpatch-through: 19 --- src/backend/replication/logical/slotsync.c | 10 ++++ .../t/040_standby_failover_slots_sync.pl | 53 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index efea6705bc1..1d654ee7f44 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -382,6 +382,16 @@ update_local_synced_slot(RemoteSlot *remote_slot, Oid remote_dbid) old_catalog_xmin != slot->data.catalog_xmin); } } + else if (slot->slotsync_skip_reason == SS_SKIP_NO_CONSISTENT_SNAPSHOT) + { + /* + * The remote slot has not advanced since the previous cycle failed to + * build a consistent snapshot, so the slot still cannot build one. + */ + Assert(slot->data.persistency == RS_TEMPORARY); + + skip_reason = SS_SKIP_NO_CONSISTENT_SNAPSHOT; + } /* Update slot sync skip stats */ update_slotsync_skip_stats(skip_reason); diff --git a/src/test/recovery/t/040_standby_failover_slots_sync.pl b/src/test/recovery/t/040_standby_failover_slots_sync.pl index f8922aaa1a2..11500ca4419 100644 --- a/src/test/recovery/t/040_standby_failover_slots_sync.pl +++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl @@ -1106,4 +1106,57 @@ $standby2->wait_for_log( $h->quit; +################################################## +# Test that a synced slot stays temporary when the standby cannot build a +# consistent snapshot across multiple sync cycles. +################################################## + +# Disable the subscription so lsub1_slot does not advance and serialize +# snapshots on standby2 while still holding back catalog_xmin on standby2. +$subscriber1->safe_psql('postgres', + "ALTER SUBSCRIPTION regress_mysub1 DISABLE"); +$primary->poll_query_until('postgres', + "SELECT NOT active FROM pg_replication_slots WHERE slot_name = 'lsub1_slot'" +); + +$primary->safe_psql('postgres', + "SELECT pg_create_logical_replication_slot('inconsistent_slot', 'test_decoding', false, false, true);" +); + +# Keep a transaction with an assigned XID open while logging xl_running_xacts +# and advancing inconsistent_slot so its restart_lsn moves to that record and +# concurrent bgwriter snapshots cannot make it consistent. Advance the slot +# twice in case a concurrent xl_running_xacts record logged before the open +# transaction was used as the candidate restart_lsn in the first call. +my $xact_h = $primary->background_psql('postgres'); +$xact_h->query_safe('BEGIN; SELECT pg_current_xact_id();'); + +$primary->safe_psql( + 'postgres', qq( + SELECT pg_log_standby_snapshot(); + BEGIN; + INSERT INTO tab_int VALUES (100); + COMMIT; + SELECT pg_replication_slot_advance('inconsistent_slot', pg_current_wal_lsn()); + SELECT pg_replication_slot_advance('inconsistent_slot', pg_current_wal_lsn()); +)); + +$xact_h->query_safe('COMMIT;'); +$xact_h->quit; + +$primary->wait_for_replay_catchup($standby2); + +$standby2->append_conf('postgresql.conf', 'sync_replication_slots = on'); +$standby2->reload; + +ok( $standby2->poll_query_until( + 'postgres', qq( + SELECT slotsync_skip_count >= 2 AND temporary AND + slotsync_skip_reason = 'no_consistent_snapshot' + FROM pg_stat_replication_slots + JOIN pg_replication_slots USING (slot_name) + WHERE slot_name = 'inconsistent_slot'; + )), + 'synced slot without a consistent snapshot stays temporary'); + done_testing(); -- 2.54.0 (Apple Git-157)
