Re: [HACKERS] logical decoding of two-phase transactions

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Nikhil Sontakke <nikhils(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, Sokolov Yura <y(dot)sokolov(at)postgrespro(dot)ru>, Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: [HACKERS] logical decoding of two-phase transactions
Date: 2018-04-03 13:56:50
Message-ID: a76f3f12-ed44-b724-1c66-d13d1920fcab@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 04/03/2018 12:40 PM, Nikhil Sontakke wrote:
> Hi,
>
>>> It's certainly a nice future goal to have it all happen automatically,
>>> but we don't know what the plugin will do.
>>
>> No, fighting too complicated APIs is not unreasonable. And we've found
>> an alternative.
>>
>
> PFA, latest patch set.
>
> The LogicalLockTransaction/LogicalUnlockTransaction API implementation
> using decode groups now has proper cleanup handling in case there's an
> ERROR while holding the logical lock.
>
> Rest of the patches are the same as yesterday.
>

Unfortunately, this does segfault for me in `make check` almost
immediately. Try

./configure --enable-debug --enable-cassert CFLAGS="-O0 -ggdb3
-DRANDOMIZE_ALLOCATED_MEMORY" && make -s clean && make -s -j4 check

and you should get an assert failure right away. Examples of backtraces
attached, not sure what exactly is the issue.

Also, I get this compiler warning:

proc.c: In function ‘AssignDecodeGroupLeader’:
proc.c:1975:8: warning: variable ‘pid’ set but not used
[-Wunused-but-set-variable]
int pid;
^~~
All of PostgreSQL successfully made. Ready to install.

which suggests we don't really need the pid variable.

> Other than this, we would want to have pgoutput support for 2PC
> decoding to be made optional? In that case we could add an option to
> "CREATE SUBSCRIPTION". This will mean adding a new
> Anum_pg_subscription_subenable_twophase attribute to Subscription
> struct and related processing. Should we go down this route?
>

I'd say yes, we need to make it opt-in (assuming we want pgoutput to
support the 2PC decoding at all).

The trouble is that while it may improve replication of two-phase
transactions, it may also require config changes on the subscriber (to
support enough prepared transactions) and furthermore the GID is going
to be copied to the subscriber.

Which means that if the publisher/subscriber (at the instance level) are
already part of the are on the same 2PC transaction, it can't possibly
proceed because the subscriber won't be able to do PREPARE TRANSACTION.

So I think we need a subscription parameter to enable/disable this,
defaulting to 'disabled'.

regards

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

Attachment Content-Type Size
backtraces.txt text/plain 3.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-04-03 13:58:55 Re: Transform for pl/perl
Previous Message Tom Lane 2018-04-03 13:56:24 Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()