Horton

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

Nikhil Sontakke <nikhils@2ndquadrant.com>

View thread View on postgr.es
From: Nikhil Sontakke <nikhils@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: 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>, Petr Jelinek <petr.jelinek@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>
Date: 2018-08-01T14:00:21Z
Lists: pgsql-hackers

Attachments

Hi,

PFA, latest patchset which incorporates the additional feedback.

>>> There's an additional test case in
>>> 0005-Additional-test-case-to-demonstrate-decoding-rollbac.patch which
>>> uses a sleep in the "change" plugin API to allow a concurrent rollback
>>> on the 2PC being currently decoded. Andres generally doesn't like this
>>> approach :-), but there are no timing/interlocking issues now, and the
>>> sleep just helps us do a concurrent rollback, so it might be ok now,
>>> all things considered. Anyways, it's an additional patch for now.
>>
>> Yea, I still don't think it's ok. The tests won't be reliable.  There's
>> ways to make this reliable, e.g. by forcing a lock to be acquired that's
>> externally held or such. Might even be doable just with a weird custom
>> datatype.
>>
>
> Ok, I will look at ways to do away with the sleep.
>

The attached patchset implements a non-sleep based approached by
sending the 2PC XID to the pg_logical_slot_get_changes() function as
an option for the test_decoding plugin. So, an example invocation
will now look like:

SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
NULL, 'skip-empty-xacts', '1', 'check-xid', '$xid2pc');

The test_decoding pg_decode_change() API if it sees a valid xid
argument will wait for it to be aborted. Another backend can then come
in and merrily abort this ongoing 2PC in the background. Once it's
aborted, the pg_decode_change API will go ahead and will hit an ERROR
in the systable scan APIs. That should take care of Andres' concern
about using sleep in the tests. The relevant tap test has been added
to this patchset.

>>> @@ -423,6 +423,16 @@ systable_getnext(SysScanDesc sysscan)
>>>       else
>>>               htup = heap_getnext(sysscan->scan, ForwardScanDirection);
>>>
>>> +     /*
>>> +      * If CheckXidAlive is valid, then we check if it aborted. If it did, we
>>> +      * error out
>>> +      */
>>> +     if (TransactionIdIsValid(CheckXidAlive) &&
>>> +                     TransactionIdDidAbort(CheckXidAlive))
>>> +                     ereport(ERROR,
>>> +                             (errcode(ERRCODE_TRANSACTION_ROLLBACK),
>>> +                              errmsg("transaction aborted during system catalog scan")));
>>> +
>>>       return htup;
>>>  }
>>
>> Don't we have to check TransactionIdIsInProgress() first? C.f. header
>> comments in tqual.c.  Note this is also not guaranteed to be correct
>> after a crash (where no clog entry will exist for an aborted xact), but
>> we probably shouldn't get here in that case - but better be safe.
>>
>> I suspect it'd be better reformulated as
>>   TransactionIdIsValid(CheckXidAlive) &&
>>   !TransactionIdIsInProgress(CheckXidAlive) &&
>>   !TransactionIdDidCommit(CheckXidAlive)
>>
>> What do you think?
>>

Modified the checks are per the above suggestion.

> I was wondering if anything else would be needed for user-defined
> catalog tables..
>

We don't need to do anything else for user-defined catalog tables
since they will also get accessed via the systable_* scan APIs.


>
> Hmm, lemme see if we can do it outside of the plugin. But note that a
> plugin might want to decode some 2PC at prepare time and another are
> "commit prepared" time.
>

The test_decoding pg_decode_filter_prepare() API implements a simple
filter strategy now. If the GID contains a substring "nodecode", then
it filters out decoding of such a 2PC at prepare time. Have added
steps to test this in the relevant test case in this patch.

I believe this patchset handles all pending issues along with relevant
test cases. Comments, further feedback appreciated.

Regards,
Nikhils
-- 
 Nikhil Sontakke                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Commits

  1. Add prepare API support for streaming transactions in logical replication.

  2. Doc: minor improvements for logical replication protocol documentation.

  3. Fix test failure in 021_twophase.pl.

  4. Refactor to make common functions in proto.c and worker.c.

  5. Unify PostgresNode's new() and get_new_node() methods

  6. Fix potential buffer overruns in proto.c.

  7. Add support for prepared transactions to built-in logical replication.

  8. Refactor function parse_subscription_options.

  9. Allow enabling two-phase option via replication protocol.

  10. Don't use Asserts to check for violations of replication protocol.

  11. Improve psql tab completion for options of subcriptions and publications

  12. Rearrange logrep worker's snapshot handling some more.

  13. doc: Update information of new messages for logical replication.

  14. ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION

  15. Allow pgoutput to send logical decoding messages.

  16. Refactor function parse_output_parameters.

  17. Avoid repeated decoding of prepared transactions after a restart.

  18. Fix an oversight in ReorderBufferFinishPrepared.

  19. Allow multiple xacts during table sync in logical replication.

  20. Fix replication of in-progress transactions in tablesync worker.

  21. Fix 'skip-empty-xacts' option in test_decoding for streaming mode.

  22. Use Enum for top level logical replication message types.

  23. Add support for streaming to built-in logical replication.

  24. Fix the logical streaming test.

  25. Implement streaming mode in ReorderBuffer.

  26. Extend the logical decoding output plugin API with stream methods.

  27. Store 2PC GID in commit/abort WAL recs for logical decoding