On Thu, Oct 8, 2026 at 6:39 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Oct 7, 2026 at 5:33 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Thu, Oct 8, 2026 at 4:08 AM Masahiko Sawada <sawada.mshk@gmail.com> 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
