Hi, On Wed, Nov11, 2026 at 01:04 AM Bertrand Drouvot wrote:
On Tue, Nov 10, 2026 at 05:17:57PM +0800, Zhijie Hou wrote: > 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. Thanks for the patch! + else if (slot->slotsync_skip_reason == SS_SKIP_NO_CONSISTENT_SNAPSHOT) Could slotsync_skip_reason be SS_SKIP_WAL_OR_ROWS_REMOVED here even if the slot is still inconsistent? This could happen if the local catalog_xmin gets ahead and the remote one later catches up?
Thanks for catching this. I confirmed that this can happen.Since any temporary slot that reaches a consistent snapshot is persisted immediately in the same cycle, we can check "else if (slot->data.persistency == RS_TEMPORARY)" here instead. Attached is the v2 patch with this change.
-- Best Regards, Zhijie Hou
From a3c160fa362a29bab385e07e51933fe5a666fdf8 Mon Sep 17 00:00:00 2001 From: Zhijie Hou <[email protected]> Date: Wed, 11 Nov 2026 23:10:44 +0800 Subject: [PATCH v2] 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 a subsequent synchronization cycle, if the remote slot has not advanced beyond the local slot, 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. Using such a slot after promotion can skip transactions committed before the consistent point, leading to data loss, or trigger an assertion failure in subsequent sync cycles. Fix this by setting the skip reason to SS_SKIP_NO_CONSISTENT_SNAPSHOT in update_local_synced_slot() when the slot is still temporary and the remote slot has not advanced beyond the local slot. Reported-by: Masahiko Sawada <[email protected]> Author: Zhijie Hou <[email protected]> Reviewed-by: Bertrand Drouvot <[email protected]> Discussion: https://postgr.es/m/cad21aoddn9qj2q5dnkw9daazvoh-9yef7jw64qphjgqi3yj...@mail.gmail.com Backpatch-through: 19 --- src/backend/replication/logical/slotsync.c | 9 ++++ .../t/040_standby_failover_slots_sync.pl | 53 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index efea6705bc1..e8564b656f9 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -382,6 +382,15 @@ update_local_synced_slot(RemoteSlot *remote_slot, Oid remote_dbid) old_catalog_xmin != slot->data.catalog_xmin); } } + else if (slot->data.persistency == RS_TEMPORARY) + { + /* + * We reach this point when a previous cycle updated confirmed_flush + * but failed to build a consistent snapshot, and the remote slot has + * not advanced beyond the local slot since then. + */ + 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 b0f4dfad6f2..8272dae95c7 100644 --- a/src/test/recovery/t/040_standby_failover_slots_sync.pl +++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl @@ -1114,4 +1114,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)
