Re: Logical replication timeout problem

On Wed, Mar 30, 2022 at 6:00 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Mar 30, 2022 at 1:24 PM wangw(dot)fnst(at)fujitsu(dot)com
> <wangw(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Tues, Mar 29, 2022 at 9:45 AM I wrote:
> > > Attach the new patch.
> >
> > Rebase the patch because the commit d5a9d86d in current HEAD.
> >
>
> Thanks, this looks good to me apart from a minor indentation change
> which I'll take care of before committing. I am planning to push this
> day after tomorrow on Friday unless there are any other major
> comments.

The patch basically looks good to me. But the only concern to me is
that once we get the patch committed, we will have to call
update_progress() at all paths in callbacks that process changes.
Which seems poor maintainability.

On the other hand, possible another solution would be to add a new
callback that is called e.g., every 1000 changes so that walsender
does its job such as timeout handling while processing the decoded
data in reorderbuffer.c. The callback is set only if the walsender
does logical decoding, otherwise NULL. With this idea, other plugins
will also be able to benefit without changes. But I’m not really sure
it’s a good design, and adding a new callback introduces complexity.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/