On 4 January 2017 at 21:20, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 31 December 2016 at 08:36, Stas Kelvich <s.kelvich@postgrespro.ru> wrote:
>> Here is resubmission of patch to implement logical decoding of two-phase transactions (instead of treating them
>> as usual transaction when commit) [1] I’ve slightly polished things and used test_decoding output plugin as client.
>
> Sounds good.
>
>> General idea quite simple here:
>>
>> * Write gid along with commit/prepare records in case of 2pc
>
> GID is now variable sized. You seem to have added this to every
> commit, not just 2PC
I've just realised that you're adding GID because it allows you to
uniquely identify the prepared xact. But then the prepared xact will
also have a regular TransactionId, which is also unique. GID exists
for users to specify things, but it is not needed internally and we
don't need to add it here. What we do need is for the commit prepared
message to remember what the xid of the prepare was and then re-find
it using the commit WAL record's twophase_xid field. So we don't need
to add GID to any WAL records, nor to any in-memory structures.
Please re-work the patch to include twophase_xid, which should make
the patch smaller and much faster too.
Please add comments to explain how and why patches work. Design
comments allow us to check the design makes sense and if it does
whether all the lines in the patch are needed to follow the design.
Without that patches are much harder to commit and we all want patches
to be easier to commit.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services