Re: [PATCH] add concurrent_abort callback for output plugin

On 30.03.21 11:12, Ajin Cherian wrote:
> I found some documentation that already was talking about concurrent
> aborts and updated that.

Thanks.

I just noticed as of PG13, concurrent_abort is part of ReorderBufferTXN,
so it seems the prepare_cb (or stream_prepare_cb) can actually figure a
concurrent abort happened (and the transaction may be incomplete).
That's good and indeed makes an additional callback unnecessary.

I recommend giving a hint to that field in the documentation as well.

> diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml
> index 80eb96d..d2f8d39 100644
> --- a/doc/src/sgml/logicaldecoding.sgml
> +++ b/doc/src/sgml/logicaldecoding.sgml
> @@ -545,12 +545,14 @@ CREATE TABLE another_catalog_table(data text) WITH (user_catalog_table = true);
> executed within that transaction. A transaction that is prepared for
> a two-phase commit using <command>PREPARE TRANSACTION</command> will
> also be decoded if the output plugin callbacks needed for decoding
> - them are provided. It is possible that the current transaction which
> + them are provided. It is possible that the current prepared transaction which
> is being decoded is aborted concurrently via a <command>ROLLBACK PREPARED</command>
> command. In that case, the logical decoding of this transaction will
> - be aborted too. We will skip all the changes of such a transaction once
> - the abort is detected and abort the transaction when we read WAL for
> - <command>ROLLBACK PREPARED</command>.
> + be aborted too. All the changes of such a transaction is skipped once

typo: changes [..] *are* skipped, plural.

> + the abort is detected and the <function>prepare_cb</function> callback is invoked.
> + This could result in a prepared transaction with incomplete changes.

... "in which case the <literal>concurrent_abort</literal> field of the
passed <literal>ReorderBufferTXN</literal> struct is set.", as a proposal?

> + This is done so that eventually when the <command>ROLLBACK PREPARED</command>
> + is decoded, there is a corresponding prepared transaction with a matching gid.
> </para>
>
> <note>

Everything else sounds good to me.

Regards

Markus