Re: [PATCH] add concurrent_abort callback for output plugin

On Tue, Mar 30, 2021 at 10:29 PM Markus Wanner <markus@bluegap.ch> wrote:

>
> 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.
>

Updated.


>
> > +     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.
>

Updated.

regards,
Ajin Cherian
Fujitsu Australia