Re: Clear logical slot's 'synced' flag on promotion of standby

On Fri, Sep 19, 2025 at 7:26 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
>
> Please find a few comments:
>
> 1)
> + * Reset all replication slots that have synced=true to synced=false.
>
> Can we please change it to:
> Reset the synced flag to false for all replication slots where it is
> currently true.
>

Changed.

> 2)
> I was wondering that since we reset the sync flag everytime we load
> slots from disk , then do we even need ResetSyncedSlots() during
> promotion? But I guess we still need it because even after promotion
> (if not restarted), existing backend sessions stay alive and it makes
> sense if they too see 'synced' as false after promotion. Is it worth
> adding this in comments atop ResetSyncedSlots() call during promotion?
>

Yes, on promotion a server could run for a long time without ever
being restarted. Do we want slots to remain in the synced state until
the next restart? I think not. I think it's best to have the logic
both in the promotion path and in the restart path.
Added a comment in xlog.c for this.

> 3)
> + if (!StandbyMode)
> + slot->data.synced = false;
>
> a)
> Do we need to mark the slot as dirty so it gets saved to disk on the
> next chance?
>
> I think ReplicationSlotSave can be skipped, as it may not be
> appropriate in the restore flow. But marking the slot dirty is
> important to avoid resetting the sync flag again on the next startup.
> A crash between marking it dirty and persisting it would still require
> a reset, but that seems acceptable. Thoughts?
>
> b)
> Also if we are marking it dirty, it makes sense to set synced to false
> only after checking if synced is true already.

Changed this accordingly. Marking it as dirty also required acquiring
the slot and releasing it afterwards. I've also not checked for
logical slots, as if the synced flag is set, it should only be for
logical slots.

Attaching v3 with the above code changes.

regards,
Ajin Cherian
Fujitsu Australia