agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Subject: [PATCH v2 2/2] Persist synchronized slot invalidations before publishing them
Date: Wed, 26 Aug 2026 05:36:40 +0000
synchronize_one_slot() publishes a remote slot's invalidation before saving
the local synchronized slot. A save failure therefore leaves the shared slot
invalid while its disk image remains valid. On the next synchronization,
drop_local_obsolete_slots() retains the local slot because the remote slot is
also invalidated. However, synchronize_one_slot() sees the local slot already
invalidated and skips the save, so the failed save is not retried directly.
Use ReplicationSlotPersistInvalidation() so the invalidated image is durable
before publication. Preserve the local restart LSN and recompute resource
horizons only after the invalidation becomes durable and visible.
A failed save now leaves the local slot valid, allowing the next synchronization
to retry. Add a primary and standby test covering the failure, a direct retry,
and an immediate restart that verifies durable invalidation.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: MiĆosz Bieniek <bieniek.milosz@proton.me>
Discussion: https://postgr.es/m/ao7u5I9OeIR72kGp%40bdtpg
Backpatch-through: 17
---
src/backend/replication/logical/slotsync.c | 10 +-
src/backend/replication/slot.c | 2 +-
.../t/057_replslot_invalidation_durability.pl | 125 ++++++++++++++++++
3 files changed, 129 insertions(+), 8 deletions(-)
10.7% src/backend/replication/logical/
3.2% src/backend/replication/
86.0% src/test/recovery/t/
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index c0403893e23..51c19c60cf9 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -829,13 +829,9 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid remote_dbid,
if (slot->data.invalidated == RS_INVAL_NONE &&
remote_slot->invalidated != RS_INVAL_NONE)
{
- SpinLockAcquire(&slot->mutex);
- slot->data.invalidated = remote_slot->invalidated;
- SpinLockRelease(&slot->mutex);
-
- /* Make sure the invalidated state persists across server restart */
- ReplicationSlotMarkDirty();
- ReplicationSlotSave();
+ ReplicationSlotPersistInvalidation(remote_slot->invalidated, false);
+ ReplicationSlotsComputeRequiredXmin(false);
+ ReplicationSlotsComputeRequiredLSN();
slot_updated = true;
}
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index b5746eb6283..02cec90a20b 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1211,7 +1211,7 @@ ReplicationSlotPersistInvalidation(ReplicationSlotInvalidationCause cause,
char path[MAXPGPATH];
Assert(MyReplicationSlot != NULL);
- Assert(MyReplicationSlot->data.persistency == RS_PERSISTENT);
+ Assert(MyReplicationSlot->data.persistency != RS_EPHEMERAL);
Assert(MyReplicationSlot->data.invalidated == RS_INVAL_NONE);
Assert(cause != RS_INVAL_NONE);
Assert(!clear_restart_lsn || cause == RS_INVAL_WAL_REMOVED);
diff --git a/src/test/recovery/t/057_replslot_invalidation_durability.pl b/src/test/recovery/t/057_replslot_invalidation_durability.pl
index 247d7b00dc1..7f9bfb2b694 100644
--- a/src/test/recovery/t/057_replslot_invalidation_durability.pl
+++ b/src/test/recovery/t/057_replslot_invalidation_durability.pl
@@ -122,4 +122,129 @@ ok(-f $restart_segment_path,
$node->stop;
+# Check that slot synchronization also persists an invalidation before
+# publishing it.
+my $primary = PostgreSQL::Test::Cluster->new('sync_primary');
+$primary->init(allows_streaming => 'logical', extra => ['--wal-segsize=1']);
+$primary->append_conf(
+ 'postgresql.conf', qq(
+autovacuum = off
+checkpoint_timeout = 1h
+max_wal_size = 64MB
+));
+$primary->start;
+$primary->safe_psql('postgres', 'CREATE EXTENSION injection_points');
+$primary->safe_psql('postgres',
+ q{SELECT pg_create_physical_replication_slot('sync_phys')});
+$primary->backup('sync_backup');
+
+my $standby = PostgreSQL::Test::Cluster->new('sync_standby');
+$standby->init_from_backup(
+ $primary, 'sync_backup',
+ has_streaming => 1,
+ has_restoring => 1);
+my $primary_connstr = $primary->connstr;
+$standby->append_conf(
+ 'postgresql.conf', qq(
+checkpoint_timeout = 1h
+hot_standby_feedback = on
+primary_slot_name = 'sync_phys'
+primary_conninfo = '$primary_connstr dbname=postgres'
+));
+$standby->start;
+$primary->wait_for_replay_catchup($standby);
+
+$primary->safe_psql(
+ 'postgres',
+ q{SELECT pg_create_logical_replication_slot(
+ 'sync_slot', 'pgoutput', false, false, true)});
+
+my $slot_synced = 'f';
+foreach (1 .. 10)
+{
+ $primary->safe_psql('postgres', 'SELECT pg_log_standby_snapshot()');
+ $primary->wait_for_replay_catchup($standby);
+ $standby->safe_psql('postgres', 'SELECT pg_sync_replication_slots()');
+ $slot_synced = $standby->safe_psql(
+ 'postgres',
+ q{
+SELECT count(*) = 1
+FROM pg_replication_slots
+WHERE slot_name = 'sync_slot'
+ AND synced
+ AND NOT temporary
+ AND invalidation_reason IS NULL
+});
+ last if $slot_synced eq 't';
+}
+is($slot_synced, 't', 'valid failover slot is synchronized');
+my $sync_restart_lsn = $standby->safe_psql(
+ 'postgres',
+ q{
+SELECT restart_lsn
+FROM pg_replication_slots
+WHERE slot_name = 'sync_slot'
+});
+
+$primary->append_conf('postgresql.conf', 'max_slot_wal_keep_size = 1MB');
+$primary->reload;
+$primary->advance_wal(8);
+$primary->wait_for_replay_catchup($standby);
+$primary->safe_psql('postgres', 'CHECKPOINT');
+
+is( $primary->safe_psql(
+ 'postgres',
+ q{
+SELECT invalidation_reason
+FROM pg_replication_slots
+WHERE slot_name = 'sync_slot'
+}),
+ 'wal_removed',
+ 'failover slot is invalidated on the primary');
+
+$standby->safe_psql(
+ 'postgres',
+ q{
+SELECT injection_points_attach(
+ 'replication-slot-save-error', 'error', 'sync_slot')
+});
+
+($ret, $stdout, $stderr) =
+ $standby->psql('postgres', 'SELECT pg_sync_replication_slots()');
+like(
+ $stderr,
+ qr/error triggered for injection point replication-slot-save-error/,
+ 'injected error prevents synchronized invalidation from being saved');
+
+is( $standby->safe_psql(
+ 'postgres',
+ q{
+SELECT invalidation_reason IS NULL
+FROM pg_replication_slots
+WHERE slot_name = 'sync_slot'
+}),
+ 't',
+ 'failed save leaves synchronized slot valid');
+
+$standby->safe_psql('postgres',
+ q{SELECT injection_points_detach('replication-slot-save-error')});
+
+$standby->safe_psql('postgres', 'SELECT pg_sync_replication_slots()');
+
+$standby->stop('immediate');
+$standby->start;
+
+is( $standby->safe_psql(
+ 'postgres',
+ qq{
+SELECT invalidation_reason, restart_lsn = '$sync_restart_lsn'
+FROM pg_replication_slots
+WHERE slot_name = 'sync_slot'
+}),
+ 'wal_removed|t',
+ 'retried synchronized invalidation and restart LSN survive restart');
+
+$standby->stop;
+$primary->stop;
+
done_testing();
--
2.34.1
--Cg2b3K2/P/2A21mh--
view thread (211+ messages) latest in thread
Message-ID: <no-message-id-1625579@localhost>
Permalink: ../no-message-id-1625579@localhost/
Also on: postgresql.org/message-id/no-message-id-1625579@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com
Subject: Re: [PATCH v2 2/2] Persist synchronized slot invalidations before publishing them
In-Reply-To: <no-message-id-1625579@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox