References: 
 <CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com>
 <87ab8e6e-ae35-154d-e3b5-3d8c83ea1eff@2ndquadrant.com>
 <20180801200123.rk6n5katv67bdvd4@alap3.anarazel.de>
 <CAMGcDxejTAJVGemUCHE-1FMAcHVNTm5qn=60s8HLCH37NCW_gw@mail.gmail.com>
 <877el38j56.fsf@ars-thinkpad>
 <20180807022739.3jg4eflrq72ypmxs@alap3.anarazel.de>
User-agent: mu4e 1.1.0; emacs 26.0.50
From: Arseny Sher <a.sher@postgrespro.ru>
To: Andres Freund <andres@anarazel.de>
Cc: Nikhil Sontakke <nikhils@2ndquadrant.com>,
 Petr Jelinek <petr.jelinek@2ndquadrant.com>,
 Robert Haas <robertmhaas@gmail.com>,
 Tomas Vondra <tomas.vondra@2ndquadrant.com>,
 David Steele <david@pgmasters.net>,
 Andrew Dunstan <andrew.dunstan@2ndquadrant.com>,
 Simon Riggs <simon@2ndquadrant.com>, Craig Ringer <craig@2ndquadrant.com>,
 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 Sokolov Yura <y.sokolov@postgrespro.ru>,
 Stas Kelvich <s.kelvich@postgrespro.ru>,
 Dmitry Dolgov <9erthalion6@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Masahiko Sawada <sawada.mshk@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] logical decoding of two-phase transactions
In-reply-to: <20180807022739.3jg4eflrq72ypmxs@alap3.anarazel.de>
Date: Tue, 07 Aug 2018 12:37:14 +0300
Message-ID: <87a7py4iwl.fsf@ars-thinkpad>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


Andres Freund <andres@anarazel.de> writes:

>> - On decoding of aborted xacts. The idea to throw an error once we
>>   detect the abort is appealing, however I think you will have problems
>>   with subxacts in the current implementation. What if subxact issues
>>   DDL and then aborted, but main transaction successfully committed?
>
> I don't see a fundamental issue here. I've not reviewed the current
> patchset meaningfully, however. Do you see a fundamental issue here?

Hmm, yes, this is not an issue for this patch because after reading
PREPARE record we know all aborted subxacts and won't try to decode
their changes. However, this will be raised once we decide to decode
in-progress transactions. Checking for all subxids is expensive;
moreover, WAL doesn't provide all of them until commit... it might be
easier to prevent vacuuming of aborted stuff while decoding needs it.
Matter for another patch, anyway.

>> - Currently we will call abort_prepared cb even if we failed to actually
>>   prepare xact due to concurrent abort. I think it is confusing for
>>   users. We should either handle this by remembering not to invoke
>>   abort_prepared in these cases or at least document this behaviour,
>>   leaving this problem to the receiver side.
>
> What precisely do you mean by "concurrent abort"?

With current patch, the following is possible:
* We start decoding of some prepared xact;
* Xact aborts (ABORT PREPARED) for any reason;
* Decoding processs notices this on catalog scan and calls abort()
  callback;
* Later decoding process reads abort record and calls abort_prepared
  callback.

--
Arseny Sher
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company