Horton

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

Ajin Cherian <itsajin@gmail.com>

View thread View on postgr.es
From: Ajin Cherian <itsajin@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, vignesh C <vignesh21@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2021-06-09T05:04:40Z
Lists: pgsql-hackers

Attachments

On Tue, Jun 8, 2021 at 4:19 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Thu, Jun 3, 2021 at 7:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Jun 2, 2021 at 4:34 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Please find attached the latest patch set v82*
> > >

Attaching patchset-v84 that addresses some of Amit's and Vignesh's comments:
This patch-set also modifies the test case added for copy_data = false
to check that two-phase
transactions are decoded correctly.

> > 2.
> > @@ -85,11 +85,16 @@ typedef struct LogicalDecodingContext
> >   bool streaming;
> >
> >   /*
> > - * Does the output plugin support two-phase decoding, and is it enabled?
> > + * Does the output plugin support two-phase decoding.
> >   */
> >   bool twophase;
> >
> >   /*
> > + * Is two-phase option given by output plugin?
> > + */
> > + bool twophase_opt_given;
> > +
> > + /*
> >   * State for writing output.
> >
> > I think we can write few comments as to why we need a separate
> > twophase parameter here? The description of twophase_opt_given can be
> > changed to: "Is two-phase option given by output plugin? This is to
> > allow output plugins to enable two_phase at the start of streaming. We
> > can't rely on twophase parameter that tells whether the plugin
> > provides all the necessary two_phase APIs for this purpose." Feel free
> > to add more to it.
> >
>
> TODO

Added comments here.

> > 3.
> > @@ -432,10 +432,19 @@ CreateInitDecodingContext(const char *plugin,
> >   MemoryContextSwitchTo(old_context);
> >
> >   /*
> > - * We allow decoding of prepared transactions iff the two_phase option is
> > - * enabled at the time of slot creation.
> > + * We allow decoding of prepared transactions when the two_phase is
> > + * enabled at the time of slot creation, or when the two_phase option is
> > + * given at the streaming start.
> >   */
> > - ctx->twophase &= MyReplicationSlot->data.two_phase;
> > + ctx->twophase &= (ctx->twophase_opt_given || slot->data.two_phase);
> > +
> > + /* Mark slot to allow two_phase decoding if not already marked */
> > + if (ctx->twophase && !slot->data.two_phase)
> > + {
> > + slot->data.two_phase = true;
> > + ReplicationSlotMarkDirty();
> > + ReplicationSlotSave();
> > + }
> >
> > Why do we need to change this during CreateInitDecodingContext which
> > is called at create_slot time? At that time, we don't need to consider
> > any options and there is no need to toggle slot's two_phase value.
> >
> >
>
> TODO

As part of the recent changes, we do turn on two_phase at create_slot time when
the subscription is created with (copy_data = false, two_phase = on).
So, this code is required.

Amit:

"1.
-   <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable
class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal> ] { <literal>PHYSICAL</literal> [
<literal>RESERVE_WAL</literal> ] | <literal>LOGICAL</literal>
<replaceable class="parameter">output_plugin</replaceable> [
<literal>EXPORT_SNAPSHOT</literal> |
<literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
] }
+   <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable
class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal> ] [ <literal>TWO_PHASE</literal> ] {
<literal>PHYSICAL</literal> [ <literal>RESERVE_WAL</literal> ] |
<literal>LOGICAL</literal> <replaceable
class="parameter">output_plugin</replaceable> [
<literal>EXPORT_SNAPSHOT</literal> |
<literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
] }

Can we do some testing of the code related to this in some way? One
random idea could be to change the current subscriber-side code just
for testing purposes to see if this works. Can we enhance and use
pg_recvlogical to test this? It is possible that if you address
comment number 13 below, this can be tested with Create Subscription
command."

Actually this is tested in the test case added when Create
Subscription with (copy_data = false) because in that case
the slot is created with the two-phase option.

Vignesh's comment:

"We could add some debug level log messages for the transaction that
will be skipped."

Updated debug messages.


regards,
Ajin Cherian
Fujitsu Australia

Commits

  1. Add prepare API support for streaming transactions in logical replication.

  2. Doc: minor improvements for logical replication protocol documentation.

  3. Fix test failure in 021_twophase.pl.

  4. Refactor to make common functions in proto.c and worker.c.

  5. Unify PostgresNode's new() and get_new_node() methods

  6. Fix potential buffer overruns in proto.c.

  7. Add support for prepared transactions to built-in logical replication.

  8. Refactor function parse_subscription_options.

  9. Allow enabling two-phase option via replication protocol.

  10. Don't use Asserts to check for violations of replication protocol.

  11. Improve psql tab completion for options of subcriptions and publications

  12. Rearrange logrep worker's snapshot handling some more.

  13. doc: Update information of new messages for logical replication.

  14. ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION

  15. Allow pgoutput to send logical decoding messages.

  16. Refactor function parse_output_parameters.

  17. Avoid repeated decoding of prepared transactions after a restart.

  18. Fix an oversight in ReorderBufferFinishPrepared.

  19. Allow multiple xacts during table sync in logical replication.

  20. Fix replication of in-progress transactions in tablesync worker.

  21. Fix 'skip-empty-xacts' option in test_decoding for streaming mode.

  22. Use Enum for top level logical replication message types.

  23. Add support for streaming to built-in logical replication.

  24. Fix the logical streaming test.

  25. Implement streaming mode in ReorderBuffer.

  26. Extend the logical decoding output plugin API with stream methods.

  27. Store 2PC GID in commit/abort WAL recs for logical decoding