On Mon, Jan 30, 2023 at 14:55 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Mon, Jan 30, 2023 at 10:36 AM wangw.f...@fujitsu.com > <wangw.f...@fujitsu.com> wrote: > > > > On Mon, Jan 30, 2023 11:37 AM Shi, Yu/侍 雨 <shiy.f...@cn.fujitsu.com> > wrote: > > > On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com > > > <wangw.f...@fujitsu.com> wrote: > > > > Yes, I think you are right. > > Fixed this problem. > > > > + /* > + * Trying to send keepalive message after every change has some > + * overhead, but testing showed there is no noticeable overhead if > + * we do it after every ~100 changes. > + */ > +#define CHANGES_THRESHOLD 100 > + > + if (++changes_count < CHANGES_THRESHOLD) > + return; > ... > + changes_count = 0; > > I think it is better to have this threshold-related code in that > caller as we have in the previous version. Also, let's modify the > comment as follows:" > It is possible that the data is not sent to downstream for a long time > either because the output plugin filtered it or there is a DDL that > generates a lot of data that is not processed by the plugin. So, in > such cases, the downstream can timeout. To avoid that we try to send a > keepalive message if required. Trying to send a keepalive message > after every change has some overhead, but testing showed there is no > noticeable overhead if we do it after every ~100 changes."
Changed as suggested. I also removed the comment atop the function update_progress_txn_cb_wrapper to be consistent with the nearby *_cb_wrapper functions. Attach the new patch. Regards, Wang Wei
v10-0001-Fix-the-logical-replication-timeout-during-proce.patch
Description: v10-0001-Fix-the-logical-replication-timeout-during-proce.patch
