Subject: Re: [HACKERS] logical decoding of two-phase transactions
To: Nikhil Sontakke <nikhils@2ndquadrant.com>,
 Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.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 mailing lists <pgsql-hackers@postgresql.org>
References: <20180719194208.j5m7qhdmqp46d6yg@alap3.anarazel.de>
 <CAMGcDxfgg8yP0MsdKRuqwihkrQF=0Mz9Se+0paNGADWaexULjQ@mail.gmail.com>
 <20180720145836.gxwhbftuoyx5h4gc@alap3.anarazel.de>
 <CAMGcDxebtC_ZO6EtLA1MgJ6sWbud3Fjtq9HsBwxxhYfoHJdANw@mail.gmail.com>
 <20180723135009.w25re6qxbb7njrbd@alap3.anarazel.de>
 <CAMGcDxdJNZVq1mX9dZbwEniigi=af8H5dt62C+q2UHwsE+BTaw@mail.gmail.com>
 <20180723141435.ercxxh3bj2uc2mjm@alap3.anarazel.de>
 <3B37B16B-A473-4352-ABCF-F23AE314D4F7@2ndquadrant.com>
 <CAMGcDxeBcMBF0KZ9qMpcb07uX6uLNO5hk=VWCQC514oZuofLmw@mail.gmail.com>
 <20180726200241.aje4dv4jsv25v4k2@alap3.anarazel.de>
 <CAMGcDxfVBiBXt4F7v58_DfqFD9xnmU8hBEiyqoypon6Ftroe_Q@mail.gmail.com>
 <a5ba1686-bb79-f2c3-568d-993e4c53b920@2ndquadrant.com>
 <CAMGcDxfWORpkaOaNhFwS4839R_w4wtDgnB8Wj2TmAQXnV78HsQ@mail.gmail.com>
 <CAMGcDxe276HwzQ1ErqMHKPf=KTvdSGgu3jYaR+j3-368+amZSA@mail.gmail.com>
 <2ba9c978-ede8-35bf-14ec-45570bf0f6aa@2ndquadrant.com>
 <CAMGcDxcBmN6jNeQkgWddfhX8HbSjQpW=Uo70iBY3P_EPdp+LTQ@mail.gmail.com>
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Openpgp: preference=signencrypt
Autocrypt: addr=petr.jelinek@2ndquadrant.com; prefer-encrypt=mutual; keydata=
 mQINBFawwSABEAC45Vua8rSggpM2kkMMmoOHz51a+fi3OXFfnrStVh80pM2/J9nSzjrIKQKf
 H5PtRnoYzSKnoUDoLOkLwDYitK3HzUUWXma+AWAqef2Y0GSZ7A3jJkD+JHOfkpS1CIpkEB/e
 WjA8Wqegwzc/OEE5111/l0Q1e6jxgZI86h8MMnm35Of1Y6Lz4kqWCyxHhaTJKTCkq66ICQ2C
 VJkbvuUoEE+Hr/I38dPHTAqIlCnycVnJPLHBQ/UYGHR/USF8qI/qJJx6tJcNAEqz69QZ/VlY
 31UyJHdYe5NtBVz+rYZCOVWfpe3crbHuq9FjAPT+xcZefdxridmMKuW8eBmmXI9iZ3EG+X8J
 IsdLWL8QoqW0lnO1otYz1QDjdOvbEpq+u5/TAU8pgtaYH2J+6/GaQ6hMLyLBkaIAtnqlLCzq
 pxXcxMsFT14P5/rZA7EbBI8RS1ZIWJBlS2oO+dtVuENLJUC0SpptaRyn/uyOIopt7hSmBekL
 SzFnet7P9qofFJY+fQEz84v0G1gvwonHGHiCGgLpbMdPtnfPzda1aiTURvvOD7w/RmX1/ogk
 TqSGO4ejVNO/28QYouxmxzy8jbbIEiLCbHpLR/M/TvYHHPURxcx+MxVl3G+d/4Ihy4abxOdW
 pe9zLIcVMvqiUFQO28/79Gfhb7G4K+L21ORB82QYkzm3M7ebXQARAQABtCNQZXRyIEplbMOt
 bmVrIDxwam1vZG9zQHBqbW9kb3MubmV0PokCQAQTAQoAKgIbAwUJBaOagAULCQgHAwUVCgkI
 CwUWAgMBAAIeAQIXgAUCVrDBqwIZAQAKCRBci2l0giH4y/QJEACfiLHyvxlwMvo3Y88Qa9QI
 i7Hu3ZW+xxojojdRd7do0nBnf2a4YstD9u6ZOLJx4f5Ijb/8LxY10NgLugmfQ7sw2W8ui6wT
 NiAeNHpr4wpT3IV+V53psOMRYPdq2zNTLO4iNxqPFPgNufR/m4FqcGe0Wo7U0wd1p0MErXTg
 EXJDVmjPDkYnzE9ejSCOxF57+o3xs6+eW9Al2m3nqP8JCzpauCr0gqpQzCaw0VZ05aLL0t9E
 B0fdm90+ydVe10Wg+I348005smuSSckBCHZqUfjjWvjwE6v/6dZ3/50ycAbi6cTe7Fo4kOSO
 sXBFXkzYGWYrEFPkzK0zDH65D98gL1+l9uokCR2t/OYVwudbUCV3cS3xRd0eFgHbXuzV+aZc
 jRjqkPfpCA2loKtrX2U739mpVSHnpIfSBf48fLg7O5dVn5ZCrydGkEzgvyynPo7IikQrIA02
 91IoSp4RisKsJpaXHwHu6DH2rDCbYFumAMgXk2osLFX6VMPoSWKVRkr87A0VLafCtwaqr6Hy
 sFE6Lmu7jP4/BDr+9A2MXK+v0yYrXqqsZN6V9856ywlZrWK3MB6jc2bKpKS2YshneKxpitNV
 0ITs2Li0OlAXOEHE0eXwI93POgLoyH+8JiNbem3orw3Mnc+7XpIY86AejwUiYl8T7EO/2tIx
 HDaXLK3BPNOdH7kCDQRWsMEgARAA6Z3NrDLdcFF/P3TD/F7WQtMALEEKn5TXSF/K2VHt21GD
 8tGY1DZhbxjLuQJeheY/2L4F4Y5WCkAoUdBXQs7nIauN+63EL88/qPdH6f3+/HZ+cmPG1E5o
 dBABCkC42eGcI5O8/9TAaFQcSzg86pdAE8v927tnc6aH4W4/Mj7RJNlQiZaz2ngCC9mr5uu+
 CysOTfKCS0llNUkExZOJzZsQU3zDkcg3+xmgtHkCjglJ006KuytHj55HR6TzaKaAW8coHGoS
 OI/BQ9KAzvlj+32/zS8FSISZSTY3spMpraADsYklejCeJOmqQ3nTrCtwlX3hTSggfr4eelKA
 hdZKs1rIimNBtX90jJc0u2A/GmC959aTSrCUcfaPRweKUuexg7jxuhXyg9br2GscD2G6W7EV
 c+3s4dOHgoJFVMBBIqTtZviaNJ63IsZIFTYJzGKKMHOioZ3naoBVav0w63waDzj82nPYMIPj
 TxnClKiecuBQQPzMaG0jTmK0lis6rO0nIXrCwca8e/3Xzvi7sykOdcz9t4ef8HrDCCkG0yDO
 o8qHy1e7EtmJD7kWffIXZMt4TZVxpiOqgWtv+heJAfmqzr8pkNLUubmgvNdnakXJOJT41Lui
 50C5OhpU9mfFCIDOeZXTQLsHA4KYaL8W/qcjv/YDmvpJtnK24y6tByWuTu7r/SMAEQEAAYkC
 JQQYAQoADwUCVrDBIAIbDAUJBaOagAAKCRBci2l0giH4yyWpD/91jd4BMN2CDYqLtU7kaoAa
 aXnRVpf0z+R8rJ4LZJX1UNbHjsdClMQLfdBuZeDsdYar4iMfwScz6fiCiRwaRpr3XT6znaM0
 wba6fjzEqVSHr3WB1jrb/TMiNaNgQJaRPhI5C7VsLuPrv5/L8gFmyjl712q7F3busGFBKrjK
 qTESMu+sHNvVeC3QyBFCe2fudbAJdaaRsUoTbAZYiowVoQHOow8eBoWY+RzQoYGuw1fls5Ck
 85N73pYHXzjfk1s49sDKTYI263h9wbGGA234biHRI3NQN6sSYWRr/u5GOwTm/tzyadRj0pnZ
 S6KyLSEL4qx8jiyniD/XLoftlvR4YWQ0BXveEElx2n3TThLL1cVtOI4qE9BZKm32euBqwLta
 k9Jc659hmZB9E9aWLo8KcLjngwqSZupoHRPZ3yAlgZkpDoTIZpMCGxGEuuxgnosRbNpOJ1fE
 nNXenYYfi87tHRUCRKeHhoKkfGZEgC9zv9Y97j75npqV2tQyh4fNQT1tGdBigtvzI4nHM1/1
 PLJtJBdRHucP76vUH9RlP9EE/Sg5DFQqsfSYPqxLW9r+ZfSEeT0APdz7TQC9MzNzUhyXvA5a
 RQQCRMXKbGaYhKkYEYiYOGqtTxN+PmyvNUBmFEcagBX2TXqEkgnI6zOnN13mB4SihJ2lne2r
 9RNiPYs/hmRxYg==
Message-ID: <380de666-a30e-8f99-1fac-0002bce60a1d@2ndquadrant.com>
Date: Fri, 25 Jan 2019 15:15:42 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: 
 <CAMGcDxcBmN6jNeQkgWddfhX8HbSjQpW=Uo70iBY3P_EPdp+LTQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

I think the difference between abort and abort prepared should be
explained better (I am not quite sure I get it myself).

> +      The required <function>abort_cb</function> callback is called whenever

Also, why is this one required when all the 2pc stuff is optional?

> +static void
> +DecodePrepare(LogicalDecodingContext *ctx, XLogRecordBuffer *buf,
> +			  xl_xact_parsed_prepare * parsed)
> +{
> +	XLogRecPtr	origin_lsn = parsed->origin_lsn;
> +	TimestampTz commit_time = parsed->origin_timestamp;
> +	XLogRecPtr	origin_id = XLogRecGetOrigin(buf->record);
> +	TransactionId xid = parsed->twophase_xid;
> +	bool		skip;
> +
> +	Assert(parsed->dbId != InvalidOid);
> +	Assert(TransactionIdIsValid(parsed->twophase_xid));
> +
> +	/* Whether or not this PREPARE needs to be skipped. */
> +	skip = DecodeEndOfTxn(ctx, buf, parsed, xid);
> +
> +	FinalizeTxnDecoding(ctx, buf, parsed, xid, skip);

Given that DecodeEndOfTxn calls SnapBuildCommitTxn, won't this make the
catalog changes done by prepared transaction visible to other
transactions (which is undesirable as they should only be visible after
it's committed) ?

> +	if (unlikely(TransactionIdIsValid(CheckXidAlive) &&
> +		!(IsCatalogRelation(scan->rs_rd) ||
> +		  RelationIsUsedAsCatalogTable(scan->rs_rd))))
> +		ereport(ERROR,
> +			(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
> +			 errmsg("improper heap_getnext call")));
> +
I think we should log the relation oid as well so that plugin developers
have easier time debugging this (for all variants of this).


-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services