RE: Rework LogicalOutputPluginWriterUpdateProgress

On Fri, Mar 10, 2023 20:17 PM Osumi, Takamichi/大墨 昂道 <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> Hi,
>
>
> On Friday, March 10, 2023 6:32 PM Wang, Wei/王 威 <wangw(dot)fnst(at)fujitsu(dot)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