On Thu, 18 Sept 2025 at 11:31, Amit Kapila <[email protected]> wrote: > > On Wed, Sep 17, 2025 at 8:19 PM Ashutosh Sharma <[email protected]> wrote: > > > > On Wed, Sep 17, 2025 at 5:14 PM Amit Kapila <[email protected]> wrote: > > > > > > On Wed, Sep 17, 2025 at 4:24 PM Hayato Kuroda (Fujitsu) > > > <[email protected]> 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. > Hi Amit,
I have written a patch which removes passing this extra parameter to the functions. I have attached the latest patch in [1]. [1]: https://www.postgresql.org/message-id/CANhcyEVFZN2Mkjs0QHshKm2_3AkQ0eufjkD12eL2MeuVkPyGbw%40mail.gmail.com Thanks, Shlok Kyal
