On Wed, Sep 17, 2025 at 8:19 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
> On Wed, Sep 17, 2025 at 5:14 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Sep 17, 2025 at 4:24 PM Hayato Kuroda (Fujitsu)
> > <kuroda.hayato@fujitsu.com> wrote:
> > >
> > > Dear Shlok,
> > >
> > > Thanks for creating the patch. Personally I prefer approach2; approach1 cannot
> > > indicate the current status of synchronization, it just shows the history.
> > > I feel approach2 has more information than approach1.
> > >
> >
> > I also think so but Ashutosh thought that it would be hacky. Ashutosh,
> > did you have an opinion on this matter after seeing the patches?
> >
>
> Yes, I’ve looked into both the patches. Approach 1 seems quite
> straightforward. In approach 2, we need to pass some additional
> arguments to update_local_sync_slot and
> update_and_persist_local_synced_slot, which makes it feel a little
> less clean compared to approach 1, where we simply add a new function
> and call it directly.
>
This is because the approach-1 doesn't show the latest value of
sync_status. I mean in the latest cycle if the sync is successful, it
won't update the stats which I am not sure is correct because users
may want to know the recent status of sync cycle. Otherwise, the patch
should be almost the same. I think we can even try to write a patch
for approach-2 without an additional out parameter in some of the
functions.
--
With Regards,
Amit Kapila.
