On Mon, 13 Mar 2023 at 08:17, wangw.f...@fujitsu.com <wangw.f...@fujitsu.com> wrote: > > On Fri, Mar 10, 2023 20:17 PM Osumi, Takamichi/大墨 昂道 > <osumi.takami...@fujitsu.com> wrote: > > Hi, > > > > > > On Friday, March 10, 2023 6:32 PM Wang, Wei/王 威 <wangw.f...@fujitsu.com> > > wrote: > > > Attach the new patch set. > > Thanks for updating the patch ! One review comment on v7-0005. > > Thanks for your comment. > > > stream_start_cb_wrapper and stream_stop_cb_wrapper don't call the pair of > > threshold check and UpdateProgressAndKeepalive unlike other write wrapper > > functions like below. But, both of them write some data to the output > > plugin, set > > the flag of did_write and thus it updates the subscriber's > > last_recv_timestamp > > used for timeout check in LogicalRepApplyLoop. So, it looks adding the pair > > to > > both functions can be more accurate, in order to reset the counter in > > changes_count on the publisher ? > > > > @@ -1280,6 +1282,8 @@ stream_start_cb_wrapper(ReorderBuffer *cache, > > ReorderBufferTXN *txn, > > > > /* Pop the error context stack */ > > error_context_stack = errcallback.previous; > > + > > + /* No progress has been made, so don't call > > UpdateProgressAndKeepalive */ > > } > > Since I think stream_start/stop_cp are different from change_cb, they don't > represent records in wal, so I think the LSNs corresponding to these two > messages are the LSNs of other records. So, we don't call the function > UpdateProgressAndKeepalive here. Also, for the reasons described in [1].#05, I > didn't reset the counter here.
As there has been no activity in this thread and it seems there is not much interest on this from the last 9 months, I have changed the status of the patch to "Returned with Feedback". Regards, Vignesh