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. [1] - https://www.postgresql.org/message-id/OS3PR01MB6275374EBE7C8CABBE6730099EAF9%40OS3PR01MB6275.jpnprd01.prod.outlook.com Regards, Wang wei
