Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

On Fri, 20 Dec 2019 at 22:30, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Dec 20, 2019 at 11:47 AM Masahiko Sawada
> <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> >
> > On Mon, 2 Dec 2019 at 17:32, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Sun, Dec 1, 2019 at 7:58 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> > > >
> > > > On Fri, Nov 22, 2019 at 01:18:11PM +0530, Dilip Kumar wrote:
> > > > > I have rebased the patch on the latest head and also fix the issue of
> > > > > "concurrent abort handling of the (sub)transaction." and attached as
> > > > > (v1-0013-Extend-handling-of-concurrent-aborts-for-streamin) along with
> > > > > the complete patch set. I have added the version number so that we
> > > > > can track the changes.
> > > >
> > > > The patch has rotten a bit and does not apply anymore. Could you
> > > > please send a rebased version? I have moved it to next CF, waiting on
> > > > author.
> > >
> > > I have rebased the patch set on the latest head.
> >
> > Thank you for working on this.
> >
> > This might have already been discussed but I have a question about the
> > changes of logical replication worker. In the current logical
> > replication there is a problem that the response time are doubled when
> > using synchronous replication because wal senders send changes after
> > commit. It's worse especially when a transaction makes a lot of
> > changes. So I expected this feature to reduce the response time by
> > sending changes even while the transaction is progressing but it
> > doesn't seem to be. The logical replication worker writes changes to
> > temporary files and applies these changes when the worker received
> > commit record (STREAM COMMIT). Since the worker sends the LSN of
> > commit record as flush LSN to the publisher after applying all
> > changes, the publisher must wait for all changes are applied to the
> > subscriber.
> >
>
> The main aim of this feature is to reduce apply lag. Because if we
> send all the changes together it can delay there apply because of
> network delay, whereas if most of the changes are already sent, then
> we will save the effort on sending the entire data at commit time.
> This in itself gives us decent benefits. Sure, we can further improve
> it by having separate workers (dedicated to apply the changes) as you
> are suggesting and in fact, there is a patch for that as well(see the
> performance results and bgworker patch at [1]), but if try to shove in
> all the things in one go, then it will be difficult to get this patch
> committed (there are already enough things and the patch is quite big
> that to get it right takes a lot of energy). So, the plan is
> something like that first we get the basic feature and then try to
> improve by having dedicated workers or things like that. Does this
> make sense to you?
>

Thank you for explanation. The plan makes sense. But I think in the
current design it's a problem that logical replication worker doesn't
receive changes (and doesn't check interrupts) during applying
committed changes even if we don't have a worker dedicated for
applying. I think the worker should continue to receive changes and
save them to temporary files even during applying changes. Otherwise
the buffer would be easily full and replication gets stuck.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services