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

Hi


On Thursday, February 11, 2021 5:10 PM Peter Smith <smithpb2250@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