On Thu, Oct 8, 2026 at 6:39 AM Masahiko Sawada <[email protected]> wrote: > > 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.
+1 > 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 too prefer Fujii-san's approach. > I've attached the patch. LGTM. thanks Shveta
