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

On Fri, Nov 13, 2026 at 7:35 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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.

While I agree that retaining the previous reason doesn't make us
persist the slot, I think it's safe only because the previous reason
is never SS_SKIP_NONE for a temporary slot at that point. That's true
today, but it depends on several things and it would be hard to modify
these code correctly in the future.

I think the root cause is that commit 788ec96d591 replaced
found_consistent_snapshot with the check of slotsync_skip_reason, but
they're not equivalent: found_consistent_snapshot is false unless the
current cycle synced the slot and found a consistent snapshot, whereas
slotsync_skip_reason is SS_SKIP_NONE unless the current cycle detected
a reason to skip. So an alternative idea would be to bring back
found_consistent_snapshot part from the commit. That way, the decision
of persisting the slot doesn't depend on the skip_reasons at all.

As for the reported reason, a temporary slot is the one we have not
confirmed to reach a consistent point yet. So I think we can simply
report no_consistent_snapshot for a temporary slot if the current
cycle didn't find a consistent snapshot, without relying on what
happened in the previous cycles.

I've attached a PoC patch.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com