On Mon, Jan 16, 2023 at 10:06 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Wed, Jan 11, 2023 at 4:11 PM wangw.f...@fujitsu.com > <wangw.f...@fujitsu.com> wrote: > > > > On Mon, Jan 9, 2023 at 13:04 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > Thanks for your comments. > > > > > One more thing, I think it would be better to expose a new callback > > > API via reorder buffer as suggested previously [2] similar to other > > > reorder buffer APIs instead of directly using reorderbuffer API to > > > invoke plugin API. > > > > Yes, I agree. I think it would be better to add a new callback API on the > > HEAD. > > So, I improved the fix approach: > > Introduce a new optional callback to update the process. This callback > > function > > is invoked at the end inside the main loop of the function > > ReorderBufferProcessTXN() for each change. In this way, I think it seems > > that > > similar timeout problems could be avoided. > > I am a bit worried about the indirections that the wrappers and hooks > create. Output plugins call OutputPluginUpdateProgress() in callbacks > but I don't see why ReorderBufferProcessTXN() needs a callback to > call OutputPluginUpdateProgress. >
Yeah, I think we can do it as we are doing the previous approach but we need an additional wrapper (update_progress_cb_wrapper()) as the current patch has so that we can add error context information. This is similar to why we have a wrapper for all other callbacks like change_cb_wrapper. -- With Regards, Amit Kapila.