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

On Fri, Mar 5, 2021 at 9:25 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:

> Thanks for the updated patch.
> Few minor comments:

> We should include two_phase in tab completion (tab-complete.c file
> psql_completion(const char *text, int start, int end) function) :
> postgres=# create subscription sub1 connection 'port=5441
> dbname=postgres' publication pub1 with (
> CONNECT COPY_DATA CREATE_SLOT ENABLED
> SLOT_NAME SYNCHRONOUS_COMMIT
>

Updated.
> +
> + <para>
> + It is not allowed to combine <literal>streaming</literal> set to
> + <literal>true</literal> and <literal>two_phase</literal> set to
> + <literal>true</literal>.
> + </para>
> +
> + </listitem>
> + </varlistentry>
> + <varlistentry>
> + <term><literal>two_phase</literal> (<type>boolean</type>)</term>
> + <listitem>
> + <para>
> + Specifies whether two-phase commit is enabled for this subscription.
> + The default is <literal>false</literal>.
> + </para>
> +
> + <para>
> + When two-phase commit is enabled then the decoded
> transactions are sent
> + to the subscriber on the PREPARE TRANSACTION. By default,
> the transaction
> + preapred on publisher is decoded as normal transaction at commit.
> + </para>
> +
> + <para>
> + It is not allowed to combine <literal>two_phase</literal> set to
> + <literal>true</literal> and <literal>streaming</literal> set to
> + <literal>true</literal>.
> + </para>
>
> It is not allowed to combine streaming set to true and two_phase set to true.
> Should this be:
> streaming option is not supported along with two_phase option.
>
> Similarly here too:
> It is not allowed to combine two_phase set to true and streaming set to true.
> Should this be:
> two_phase option is not supported along with streaming option.

Reworded this with a small change.

>
> Few indentation issues are present, we can run pgindent:
> +extern void logicalrep_write_prepare(StringInfo out, ReorderBufferTXN *txn,
> +
> XLogRecPtr prepare_lsn);
> +extern void logicalrep_read_prepare(StringInfo in,
> +
> LogicalRepPreparedTxnData *prepare_data);
> +extern void logicalrep_write_commit_prepared(StringInfo out,
> ReorderBufferTXN* txn,
> +
> XLogRecPtr commit_lsn);
>
> ReorderBufferTXN * should be ReorderBufferTXN*

Changed accordingly.

Created new patch v53:
* Rebased to HEAD (this resulted in removing patch 0001) and reduced
patch-set to 4 patches.
* Removed the changes for making "stream prepare" optional from
required. Will create a new patch and thread for this.

regards,
Ajin Cherian
Fujitsu Australia