Re: Re: Assertion failure in update_local_synced_slot(), slotsync.c:364

Hi,

Thank you for the patch. I tried v2 and it looks good to me.

What I did:

- Applied v2 to current master (ef1b740441c). It applies cleanly (git apply
--check) and builds without warnings (--enable-cassert --enable-tap-tests).
- t/
https://www.google.com/url?q=http://040_standby_failover_slots_sync.pl&source=gmail&ust=1794597551024000&sa=E
passes with the patch (39 tests), and I ran it 5 more times with the same
result.
- make check passes (243 tests). The whole src/test/recovery TAP suite
passes with the patch (59 files, 691 tests; 7 files that need injection
points were skipped, as my build has none).
- I applied only the t/040 hunk to unpatched master. The new test fails
there: the poll for "slotsync_skip_count >= 2 AND temporary AND
slotsync_skip_reason = 'no_consistent_snapshot'" times out and returns
false (test 39 of 39). So the new test does catch the problem. With the
full patch it passes. As expected, the failure comes as a timeout and not
as a fast failure.

The new "else if (slot->data.persistency == RS_TEMPORARY)" branch looks
right to me.

I have not tested the back branches, a meson build,
pg_sync_replication_slots() with a stopped primary slot, or the original
assertion failure scenario by hand, so I can not say anything about those.

Thanks again,
Joao Rito

On Wed, Nov 11, 2026 12:13 PM, Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:

> 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