pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Stas Kelvich <s.kelvich@postgrespro.ru>
To: Nikhil Sontakke <nikhils@2ndquadrant.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: Craig Ringer <craig@2ndquadrant.com>
Cc: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Cc: Sokolov Yura <y.sokolov@postgrespro.ru>
Cc: Andres Freund <andres@anarazel.de>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: [HACKERS] logical decoding of two-phase transactions
Date: Thu, 8 Feb 2018 15:43:23 +0300
Message-ID: <11AEF68F-14A2-4662-AAEE-87422ADA3864@postgrespro.ru> (raw )
In-Reply-To: <CAMGcDxfc0yWmdkn28UBkyEjdCWqXmNgS2v=cjOgWD39mvvoY+w@mail.gmail.com >
References: <A434797D-1283-42C5-8CE5-7F1CD0B68F36@postgrespro.ru >
<CAMsr+YF4QfAKzsfHQ7MS+5NG3aY-NG3T_DdhcdPEPkWD4fo7fQ@mail.gmail.com >
<CAMGcDxf83P5SGnGH52=_0wRP9pO6uRWCMRwAA0nxKtZvir2_vQ@mail.gmail.com >
<CAMsr+YGX1z5t_MccrJxd4VJPgZKx17EsAQcAQ_1iJ4dKDp0LOg@mail.gmail.com >
<CAMGcDxeDd4uB+tvaYrtjLgykeOpa-5nwPetNM+prCQ=wpPmPaQ@mail.gmail.com >
<CAMsr+YH_x7Lrz3arUewzAk4=FZ-+hdum-eW7gZJBAEmzo937eg@mail.gmail.com >
<e55b1240-df0b-c5eb-bcdb-18902cdf42f8@2ndquadrant.com >
<CAMsr+YGE84n5_1Z=xNwaRxS3Fx6WS7=FLcYtezt1wRx7VNjW0Q@mail.gmail.com >
<CAMGcDxfin6iYuHN1_21Z7YYvPDU9qLgfBSBYti1KWiUUpJBr7g@mail.gmail.com >
<CAMGcDxf0YDPxgG3sU=0k8zTZniEe2RhT90v4BP__3a1P4iHpEQ@mail.gmail.com >
<45525c30-4002-861e-a82c-3b95b19d1514@2ndquadrant.com >
<f351fc25-7b4d-3bf3-0398-d8a55bb3bd87@2ndquadrant.com >
<CAMGcDxc0+17=5jZ7CXPpZFT9ugwLNXzPcht2J9EbaR-dXrDXCQ@mail.gmail.com >
<CANP8+jKwhA9cofuUPP_OxEgy_2k3-LHBxw9E57MnBiS1U-3xVw@mail.gmail.com >
<CAMGcDxca0zLfpMWTUV+1QT2NjZw+w2Zfzc2NKGfOH41UwFjQmw@mail.gmail.com >
<dec39ab4-cc36-e01c-7a00-0087fe8644da@2ndquadrant.com >
<CAMGcDxfxEodz8+THwaXtjpW5WuBpeYp1t-UT9Wa-NfNuWaBVGA@mail.gmail.com >
<CAMGcDxdDqZHyi=WJcOZUZ3J-oY6cKvWhHJ2dS+4qex8xA8Jz6A@mail.gmail.com >
<CAMGcDxdDCQipHyBeqT6crJE4WpFKT9LxV1TY2151Jy9udnMLFQ@mail.gmail.com >
<CAMGcDxfc0yWmdkn28UBkyEjdCWqXmNgS2v=cjOgWD39mvvoY+w@mail.gmail.com >
Hi!
Thanks for working on this patch.
Reading through patch I’ve noticed that you deleted call to SnapBuildCommitTxn()
in DecodePrepare(). As you correctly spotted upthread there was unnecessary
code that marked transaction as running after decoding of prepare. However call
marking it as committed before decoding of prepare IMHO is still needed as
SnapBuildCommitTxn does some useful thing like setting base snapshot for parent
transactions which were skipped because of SnapBuildXactNeedsSkip().
E.g. current code will crash in assert for following transaction:
BEGIN;
SAVEPOINT one;
CREATE TABLE test_prepared_savepoints (a int);
PREPARE TRANSACTION 'x';
COMMIT PREPARED 'x';
:get_with2pc_nofilter
:get_with2pc_nofilter <- second call will crash decoder
With following backtrace:
frame #3: 0x000000010dc47b40 postgres`ExceptionalCondition(conditionName="!(txn->ninvalidations == 0)", errorType="FailedAssertion", fileName="reorderbuffer.c", lineNumber=1944) at assert.c:54
frame #4: 0x000000010d9ff4dc postgres`ReorderBufferForget(rb=0x00007fe1ab832318, xid=816, lsn=35096144) at reorderbuffer.c:1944
frame #5: 0x000000010d9f055c postgres`DecodePrepare(ctx=0x00007fe1ab81b918, buf=0x00007ffee2650408, parsed=0x00007ffee2650088) at decode.c:703
frame #6: 0x000000010d9ef718 postgres`DecodeXactOp(ctx=0x00007fe1ab81b918, buf=0x00007ffee2650408) at decode.c:310
That can be fixed by calling SnapBuildCommitTxn() in DecodePrepare()
which I believe is safe because during normal work prepared transaction
holds relation locks until commit/abort and in between nobody can access
altered relations (or just I don’t know such situations — that was the reason
why i had marked that xids as running in previous versions).
> On 6 Feb 2018, at 15:20, Nikhil Sontakke <nikhils@2ndquadrant.com> wrote:
>
> Hi all,
>
>>
>> PFA, patch which applies cleanly against latest git head. I also
>> removed unwanted newlines and took care of the cleanup TODO about
>> making ReorderBufferTXN structure using a txn_flags field instead of
>> separate booleans for various statuses like has_catalog_changes,
>> is_subxact, is_serialized etc. The patch uses this txn_flags field for
>> the newer prepare related info as well.
>>
>> "make check-world" passes ok, including the additional regular and tap
>> tests that we have added as part of this patch.
>>
>
> PFA, latest version of this patch.
>
> This latest version takes care of the abort-while-decoding issue along
> with additional test cases and documentation changes.
>
>
--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
view thread (644+ messages) latest in thread
Message-ID: <11AEF68F-14A2-4662-AAEE-87422ADA3864@postgrespro.ru>
Permalink: ../11AEF68F-14A2-4662-AAEE-87422ADA3864@postgrespro.ru/
Also on: postgresql.org/message-id/11AEF68F-14A2-4662-AAEE-87422ADA3864@postgrespro.ru
copy link · copy postgr.es
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: s.kelvich@postgrespro.ru, nikhils@2ndquadrant.com, peter.eisentraut@2ndquadrant.com, simon@2ndquadrant.com, craig@2ndquadrant.com, petr.jelinek@2ndquadrant.com, y.sokolov@postgrespro.ru, andres@anarazel.de, 9erthalion6@gmail.com, tgl@sss.pgh.pa.us, sawada.mshk@gmail.com, robertmhaas@gmail.com
Subject: Re: [HACKERS] logical decoding of two-phase transactions
In-Reply-To: <11AEF68F-14A2-4662-AAEE-87422ADA3864@postgrespro.ru>
* 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