Dear Kuroda-san, On Thu, Jun 26, 2025 at 6:46 AM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.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
