Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly

Dear Kuroda-san,

On Thu, Jun 26, 2025 at 6:46 AM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Alexander,
> >
> > Good idea. But I think we should associate the "updated" flag
> > directly to the fact that one slot (no matter logical or physical)
> > changed its last_saved_restart_lsn. See the attached patch. I'm
> > going to push it if no objections.
>
> + /*
> + * Track if we're going to update slot's last_saved_restart_lsn.
> + * We need this to know if we need to recompute the required LSN.
> + */
> + if (s->last_saved_restart_lsn != s->data.restart_lsn)
> + last_saved_restart_lsn_updated = true;
>
> I feel no need to set to true if last_saved_restart_lsn_updated is already true.
> Other than that it's OK for me.

Thank you for your feedback.

Regarding last_saved_restart_lsn_updated, I think the opposite. I
think we should check if last_saved_restart_lsn_updated is set already
only if it could promise us some economy of resources. In our case
the main check only compares two fields of slot. And that fields are
to be accessed anyway. So, we are not going to save any RAM accesses.
Therefore, checking for last_saved_restart_lsn_updated seems like
unnecessary code complication (and I don't see we're doing that in
other places). So, I'm going to push this patch "as is".

------
Regards,
Alexander Korotkov
Supabase