pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Date: Thu, 26 Sep 2019 20:07:52 +0200
Message-ID: <20190926175845.pau2qocmlffd37qg@development> (raw)
In-Reply-To: <CAA4eK1KtxLpSp2rP6Rt8izQnPmhiA=2QUpLk+voagTjKowc0HA@mail.gmail.com>
References: <688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com>
	<20190902220650.GA1868@alvherre.pgsql>
	<CAA4eK1KtxLpSp2rP6Rt8izQnPmhiA=2QUpLk+voagTjKowc0HA@mail.gmail.com>

Hi,

Attached is an updated patch series, rebased on current master. It does
fix one memory accounting bug in ReorderBufferToastReplace (the code was
not properly updating the amount of memory).

I've also included the patch series with decoding of 2PC transactions,
which this depends on. This way we have a chance of making the cfbot
happy. So parts 0001-0004 and 0009-0014 are "this" patch series, while
0005-0008 are the extra pieces from the other patch.

I've done it like this because the initial parts are independent, and so
might be committed irrespectedly of the other patch series. In practice
that's only reasonable for 0001, which adds the memory limit - the rest
is infrastucture for the streaming of in-progress transactions.

On Wed, Sep 25, 2019 at 06:55:01PM +0530, Amit Kapila wrote:
>On Tue, Sep 3, 2019 at 4:30 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>>
>> In the interest of moving things forward, how far are we from making
>> 0001 committable?  If I understand correctly, the rest of this patchset
>> depends on https://commitfest.postgresql.org/24/944/ which seems to be
>> moving at a glacial pace (or, actually, slower, because glaciers do
>> move, which cannot be said of that other patch.)
>>
>
>I am not sure if it is completely correct that the other part of the
>patch is dependent on that CF entry.  I have studied both the threads
>(not every detail) and it seems to me it is dependent on one of the
>patches from that series which handles concurrent aborts.  It is patch
>0003-Gracefully-handle-concurrent-aborts-of-uncommitted-t.Jan4.patch
>from what the Nikhil has posted on that thread [1].  Am, I wrong?
>

You're right - the part handling aborts is the only part required. There
are dependencies on some other changes from the 2PC patch, but those are
mostly refactorings that can be undone (e.g. switch from independent
flags to a single bitmap in reorderbuffer).

>So IIUC, the problem of concurrent aborts is that if we allow catalog
>scans for in-progress transactions, then we might get wrong answers in
>cases where somebody has performed Alter-Abort-Alter which is clearly
>explained with an example in email [2].  To solve that problem Nikhil
>seems to have written a patch [1] which detects these concurrent
>aborts during a system table scan and then aborts the decoding of such
>a transaction.
>
>Now, the problem is that patch has written considering 2PC
>transactions and might not deal with all cases for in-progress
>transactions especially when sub-transactions are involved as alluded
>by Arseny Sher [3].  So, the problem seems to be for cases when some
>sub-transaction aborts, but the main transaction still continued and
>we try to decode it.  Nikhil's patch won't be able to deal with it
>because I think it just checks top-level xid whereas for this we need
>to check all-subxids which I think is possible now as Tomas seems to
>have written WAL for each xid-assignment.  It might or might not be
>the best solution to check the status of all-subxids, but I think
>first we need to agree that the problem is just for concurrent aborts
>and that we can solve it by using some part of the technology being
>developed as part of patch "Logical decoding of two-phase
>transactions" (https://commitfest.postgresql.org/24/944/) rather than
>the entire patchset.
>
>I hope I am not saying something very obvious here and it helps in
>moving this patch forward.
>

No, that's a good question, and I'm not sure what the answer is at the
moment. My understanding was that the infrastructure in the 2PC patch is
enough even for subtransactions, but I might be wrong. I need to think
about that for a while.

Maybe we should focus on the 0001 part for now - it can be committed
indepently and does provide useful feature.

regards

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

Attachments:

  0001-Add-logical_work_mem-to-limit-ReorderBuffer-20190926.patch.gz (application/gzip, 10.4K) download

  0002-Immediately-WAL-log-assignments-20190926.patch.gz (application/gzip, 6.6K) download

  0003-Issue-individual-invalidations-with-wal_lev-20190926.patch.gz (application/gzip, 4.9K) download

  0004-Extend-the-output-plugin-API-with-stream-me-20190926.patch.gz (application/gzip, 6.1K) download

  0005-Cleaning-up-of-flags-in-ReorderBufferTXN-st-20190926.patch.gz (application/gzip, 2.5K) download

  0006-Support-decoding-of-two-phase-transactions--20190926.patch.gz (application/gzip, 9.0K) download

  0007-Gracefully-handle-concurrent-aborts-of-unco-20190926.patch.gz (application/gzip, 4.4K) download

  0008-Teach-test_decoding-plugin-to-work-with-2PC-20190926.patch.gz (application/gzip, 5.9K) download

  0009-Implement-streaming-mode-in-ReorderBuffer-20190926.patch.gz (application/gzip, 12.7K) download

  0010-Add-support-for-streaming-to-built-in-repli-20190926.patch.gz (application/gzip, 19.8K) download

  0011-Track-statistics-for-streaming-spilling-20190926.patch.gz (application/gzip, 4.2K) download

  0012-Enable-streaming-for-all-subscription-TAP-t-20190926.patch.gz (application/gzip, 2.1K) download

  0013-BUGFIX-set-final_lsn-for-subxacts-before-cl-20190926.patch.gz (application/gzip, 631B) download

  0014-Add-TAP-test-for-streaming-vs.-DDL-20190926.patch.gz (application/gzip, 1.6K) download

view thread (536+ messages)  latest in thread

Message-ID: <20190926175845.pau2qocmlffd37qg@development>
Permalink:  ../20190926175845.pau2qocmlffd37qg@development/
Also on:    postgresql.org/message-id/20190926175845.pau2qocmlffd37qg@development

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tomas.vondra@2ndquadrant.com, amit.kapila16@gmail.com, alvherre@2ndquadrant.com
  Subject: Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
  In-Reply-To: <20190926175845.pau2qocmlffd37qg@development>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox