On Wed, Oct 7, 2026 at 5:33 PM Fujii Masao <[email protected]> wrote:
>
> On Thu, Oct 8, 2026 at 4:08 AM Masahiko Sawada <[email protected]> wrote:
> > Pushed.
>
> The buildfarm member skink reported a failure in the test added by commit
> 1f2245371e3 [1].
>
> [23:05:02.489](9.575s) not ok 45 - no synced slot is left behind on
> standby5 after the re-creation test
> [23:05:02.489](0.000s) # Failed test 'no synced slot is left behind
> on standby5 after the re-creation test'
>
> The test expects the standby's synced slot to disappear after the
> corresponding slot is dropped on the primary. But, in the failed test
> case, the test found one slot instead of zero.
>
> The slot can become persistent before the primary drops it. In that case,
> WAL replay invalidates the standby slot but does not remove it, leaving one
> slot behind. This seems to explain the slot-count failure.
Right. The sync_slot was created on the standby from the re-created
remote slot as we expect, but it then became sync-ready in the same
pg_sync_replication_slots() call. I expected that after detecting the
disable/re-enable, the retry would re-create the slot but not be able
to persist it because the standby's xmin
is ahead of the remote slot's. But that's not necessarily true.
> To address this, how about running pg_sync_replication_slots() once more
> as follows? This extra sync cycle removes the obsolete slot.
>
> $primary->wait_for_replay_catchup($standby5);
> $psql_sync_slot->quit;
> + # The slot may have become sync-ready before it was dropped on the
> + # primary. In that case, replay invalidates it, and a new sync cycle
> + # must remove it from the standby.
> + $standby5->safe_psql('postgres',
> + qq[select pg_sync_replication_slots()]);
> $primary->safe_psql('postgres',
> qq[select pg_drop_replication_slot('test_slot4')]);
> wait_for_logical_decoding_disabled($primary);
I agree that it's a reasonable fix. This can handle both cases where
the synced slot is persistent and not persistent: if the synced slot
was persisted, drop_local_obsolete_slots() removes it now that the
remote slot is gone, and if it stayed temporary it has already been
removed by the time the background session finished, so the extra call
is just a no-op.
Another option would be to make it deterministic the other way around:
advance the remote slot on the primary until the synced slot becomes
sync-ready, then clean up. That's what
040_standby_failover_slots_sync.pl does for its retry case, but there
the subscription keeps consuming the remote slot for us. Nothing
consumes sync_slot here, so we would need a loop calling
pg_log_standby_snapshot() and pg_logical_slot_get_changes() until the
slot is persisted. I prefer Fujii-san's approach.
I've attached the patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
From 05c0d2e5a6b2c78a1b7535513cd092e67ea0086e Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <[email protected]>
Date: Wed, 7 Oct 2026 17:51:53 -0700
Subject: [PATCH v1] Stabilize recently added tests in
051_effective_wal_level.pl.
Commit 1f2245371e3 added a test for the case where logical decoding
is disabled and re-enabled while synchronizing a replication slot. Its
cleanup assumed the slot re-created on retry would still be temporary,
and so would be gone once the corresponding remote slot was
dropped. That isn't guaranteed: the catalog_xmin reserved on the
standby is not necessarily ahead of the remote slot's. In that case
the slot is persisted on the first retry, and the subsequent logical
decoding deactivation invalidates it on the standby rather than
dropping it, so the check that no synced slot is left behind fails.
Stabilize the cleanup by calling an extra pg_sync_replication_slots()
after dropping the remote slot and before disabling logical decoding on
the primary.
Per buildfarm member skink.
Reported-by: Fujii Masao <[email protected]>
Suggsted-by: Fujii Masao <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/cahgqgwfwxasacrnhhjgedk8nwylw4blldhieiksqbqd+tzj...@mail.gmail.com
Backpatch-through: 19
---
src/test/recovery/t/051_effective_wal_level.pl | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/test/recovery/t/051_effective_wal_level.pl b/src/test/recovery/t/051_effective_wal_level.pl
index 3b70a0722a9..4aa26ec786b 100644
--- a/src/test/recovery/t/051_effective_wal_level.pl
+++ b/src/test/recovery/t/051_effective_wal_level.pl
@@ -687,10 +687,9 @@ select injection_points_wakeup('replication-slot-create-begin');
or die
"timed out waiting for the slot to be re-created from the re-created remote slot";
- # The slot created on retry might not be persisted until the remote slot
- # catches up with the catalog_xmin computed locally. Drop the remote slot
- # to let the slot synchronization finish, keeping logical decoding enabled
- # with another slot as the slot synchronization requires it.
+ # Drop the remote slot so that the slot synchronization has nothing left
+ # to retry, keeping logical decoding enabled with another slot as the
+ # slot synchronization requires it.
$primary->safe_psql(
'postgres', qq[
select pg_create_logical_replication_slot('test_slot4', 'test_decoding');
@@ -698,6 +697,12 @@ select pg_drop_replication_slot('sync_slot');
]);
$primary->wait_for_replay_catchup($standby5);
$psql_sync_slot->quit;
+
+ # The slot created on retry may or may not have been persisted by the
+ # time the remote slot was dropped. One more synchronization cycle
+ # drops the local slot either way.
+ $standby5->safe_psql('postgres', qq[select pg_sync_replication_slots()]);
+
$primary->safe_psql('postgres',
qq[select pg_drop_replication_slot('test_slot4')]);
wait_for_logical_decoding_disabled($primary);
--
2.55.0
