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