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

Hi

On Thursday, February 11, 2021 5:10 PM Peter Smith <smithpb2250(at)gmail(dot)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