Re: [HACKERS] logical decoding of two-phase transactions - Mailing list pgsql-hackers
From | Masahiko Sawada |
---|---|
Subject | Re: [HACKERS] logical decoding of two-phase transactions |
Date | |
Msg-id | CAD21AoAUicUSqEGSQ-3biofJhLZkyccV-yzeWxwszn0wis+7sg@mail.gmail.com Whole thread Raw |
In response to | Re: [HACKERS] logical decoding of two-phase transactions (Peter Smith <smithpb2250@gmail.com>) |
Responses |
Re: [HACKERS] logical decoding of two-phase transactions
Re: [HACKERS] logical decoding of two-phase transactions |
List | pgsql-hackers |
On Mon, Nov 9, 2020 at 3:23 PM Peter Smith <smithpb2250@gmail.com> wrote: > > > > 4. > > > +static void > > > +apply_handle_prepare_txn(LogicalRepPrepareData * prepare_data) > > > +{ > > > + Assert(prepare_data->prepare_lsn == remote_final_lsn); > > > + > > > + /* The synchronization worker runs in single transaction. */ > > > + if (IsTransactionState() && !am_tablesync_worker()) > > > + { > > > + /* End the earlier transaction and start a new one */ > > > + BeginTransactionBlock(); > > > + CommitTransactionCommand(); > > > + StartTransactionCommand(); > > > > > > There is no explanation as to why you want to end the previous > > > transaction and start a new one. Even if we have to do so, we first > > > need to call BeginTransactionBlock before CommitTransactionCommand. > > Done > > --- > > Also... > > pgindent has been run for all patches now. > > The latest of all six patches are again reunited with a common v18 > version number. > I've looked at the patches and done some tests. Here is my comment and question I realized during testing and reviewing. +static void +DecodePrepare(LogicalDecodingContext *ctx, XLogRecordBuffer *buf, + xl_xact_parsed_prepare *parsed) +{ + XLogRecPtr origin_lsn = parsed->origin_lsn; + TimestampTz commit_time = parsed->origin_timestamp; static void DecodeAbort(LogicalDecodingContext *ctx, XLogRecordBuffer *buf, - xl_xact_parsed_abort *parsed, TransactionId xid) + xl_xact_parsed_abort *parsed, TransactionId xid, bool prepared) { int i; + XLogRecPtr origin_lsn = InvalidXLogRecPtr; + TimestampTz commit_time = 0; + XLogRecPtr origin_id = XLogRecGetOrigin(buf->record); - for (i = 0; i < parsed->nsubxacts; i++) + if (parsed->xinfo & XACT_XINFO_HAS_ORIGIN) { - ReorderBufferAbort(ctx->reorder, parsed->subxacts[i], - buf->record->EndRecPtr); + origin_lsn = parsed->origin_lsn; + commit_time = parsed->origin_timestamp; } In the above two changes, parsed->origin_timestamp is used as commit_time. But in DecodeCommit() we use parsed->xact_time instead. Therefore it a transaction didn't have replorigin_session_origin the timestamp of logical decoding out generated by test_decoding with 'include-timestamp' option is invalid. Is it intentional? --- + if (is_commit) + txn->txn_flags |= RBTXN_COMMIT_PREPARED; + else + txn->txn_flags |= RBTXN_ROLLBACK_PREPARED; + + if (rbtxn_commit_prepared(txn)) + rb->commit_prepared(rb, txn, commit_lsn); + else if (rbtxn_rollback_prepared(txn)) + rb->rollback_prepared(rb, txn, commit_lsn); RBTXN_COMMIT_PREPARED and RBTXN_ROLLBACK_PREPARED are used only here and it seems to me that it's not necessarily necessary. --- + /* + * If this is COMMIT_PREPARED and the output plugin supports + * two-phase commits then set the prepared flag to true. + */ + prepared = ((info == XLOG_XACT_COMMIT_PREPARED) && ctx->twophase) ? true : false; We can write instead: prepared = ((info == XLOG_XACT_COMMIT_PREPARED) && ctx->twophase); + /* + * If this is ABORT_PREPARED and the output plugin supports + * two-phase commits then set the prepared flag to true. + */ + prepared = ((info == XLOG_XACT_ABORT_PREPARED) && ctx->twophase) ? true : false; The same is true here. --- 'git show --check' of v18-0002 reports some warnings. Regards, -- Masahiko Sawada EnterpriseDB: https://www.enterprisedb.com/
pgsql-hackers by date: