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

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Erik Rijkers <er(at)xs4all(dot)nl>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Date: 2020-07-15 03:59:03
Message-ID: CAFiTN-spySLsMizutzg2apuKhyhemhwsDCn_gsBvf1j+p7jkwA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 14, 2020 at 5:40 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Jul 13, 2020 at 4:09 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 13, 2020 at 4:00 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > >
> > > Okay, in that case, we can do both enable and disable streaming in
> > > this function itself rather than allow the caller to later modify it.
> > > I suggest similarly we can enable/disable it for SQL API in
> > > pg_decode_startup via output_plugin_options. This way it will look
> > > consistent for both SQL APIs and for command-based replication. If we
> > > can do so, then probably adding an Assert for Consistent Snapshot
> > > while performing streaming should be okay.
> >
> > Sounds good to me.
> >
>
> Please find the latest patches. I have made changes only in the
> subscriber-side patches (0007 and 0008 as per the current patch-set).
> The main changes are:
> 1. As discussed above, remove SendFeedback call from apply_handle_stream_commit
> 2. In SharedFilesetInit, ensure to register callback once
> 3. In stream_open_file, change slight handling around MemoryContexts
> 4. Merged the subscriber-side patches.
> 5. Added/Edited comments in 0007 and 0008.

I have reviewed your changes and those look good to me, please find
the latest version of the patch set. The major changes
- A couple of review comments fixed suggested upthread in 0003 and 0005.
- Handle the case of stop streaming until we reach to the
start_decoding_at LSN in 0005
- Simplified the 0006 by avoiding sending the transaction with
incomplete changes and added the comment atop
ReorderBufferLargestTopTXN
- Moved 0010 as 0007 and handled pending comments in the same.
- In 0009 I have fixed a couple of defects mentioned above. And, one
additional defect that is, if we do alter subscription streaming
off/on then it was not working.
- In 0009 sending the origin id.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
v33.tar application/x-tar 283.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2020-07-15 04:00:34 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Previous Message David Rowley 2020-07-15 03:52:17 Re: Parallel Seq Scan vs kernel read ahead