Hi
On Thursday, February 11, 2021 5:10 PM Peter Smith <smithpb2...@gmail.com> wrote: > Please find attached the new 2PC patch set v39* I started to review the patchset so, let me give some comments I have at this moment. (1) File : v39-0007-Support-2PC-txn-tests-for-concurrent-aborts.patch Modification : @@ -620,6 +666,9 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, } txndata->xact_wrote_changes = true; + /* For testing concurrent aborts */ + test_concurrent_aborts(data); + class_form = RelationGetForm(relation); tupdesc = RelationGetDescr(relation); Comment : There are unnecessary whitespaces in comments like above in v37-007 Please check such as pg_decode_change(), pg_decode_truncate(), pg_decode_stream_truncate() as well. I suggest you align the code formats by pgindent. (2) File : v39-0006-Support-2PC-txn-Subscription-option.patch @@ -213,6 +219,15 @@ parse_subscription_options(List *options, *streaming_given = true; *streaming = defGetBoolean(defel); } + else if (strcmp(defel->defname, "two_phase") == 0 && twophase) + { + if (*twophase_given) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + *twophase_given = true; + *twophase = defGetBoolean(defel); + } You can add this test in subscription.sql easily with double twophase options. When I find something else, I'll let you know. Best Regards, Takamichi Osumi