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

On Fri, Nov 13, 2026 at 7:29 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> On Fri, Nov 13, 2026 at 04:45:00PM +0530, Amit Kapila wrote:
> >
> > + else if (slot->data.persistency == RS_TEMPORARY)
> > + {
> > + /*
> > + * We reach this point when a previous cycle updated confirmed_flush
> > + * but failed to build a consistent snapshot, and the remote slot has
> > + * not advanced beyond the local slot since then.
> > + */
> > + skip_reason = SS_SKIP_NO_CONSISTENT_SNAPSHOT;
> > + }
> >
> > Is it better to directly use the slot's skip_reason rather than
> > assuming SS_SKIP_NO_CONSISTENT_SNAPSHOT?
>
> That could work but what about the case I mentioned upthread where the local
> catalog_xmin gets ahead and the remote later catches up? I think that that would
> keep slotsync_skip_reason as SS_SKIP_WAL_OR_ROWS_REMOVED, although the slot is
> still inconsistent due to the missing snapshot.
>

Right the reason can remain as SS_SKIP_WAL_OR_ROWS_REMOVED. Even in
the first cycle, the reason could be either SS_SKIP_WAL_NOT_FLUSHED or
SS_SKIP_WAL_OR_ROWS_REMOVED and in that case also the caller
update_and_persist_local_synced_slot() won't persist the slot. So, I
don't see a problem with retaining the previous reason when we didn't
perform anything in a particular sync cycle.

> > I understand today it is correct but tomorrow if there are more reasons
>
> Maybe we could keep v2 and assert the two possible previous reasons, so any new
> reason would need explicit handling?
>

Hmm, yeah that kind of forceful handling can also work but when we
don't see any harm in retaining the previous skip reason then why not
do that instead?

--
With Regards,
Amit Kapila.