Re: Fix slot synchronization with two_phase decoding enabled

Peter Smith <smithpb2250@gmail.com>

From: Peter Smith <smithpb2250@gmail.com>
To: Ajin Cherian <itsajin@gmail.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, shveta malik <shveta.malik@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-06-11T22:30:26Z
Lists: pgsql-hackers
On Wed, Jun 11, 2025 at 8:16 PM Ajin Cherian <itsajin@gmail.com> wrote:
>
> On Fri, Jun 6, 2025 at 5:07 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> >
> > Attached v18 patch.
> >  - patch-001: modified error messages as suggested above.
> >  - patch-002: improved pg_dump docs as per Shveta's off-list suggestions.
> >
> > [1] https://www.postgresql.org/message-id/CAA4eK1%2BB067G8mUJzKUEjc5KSkYq6z0utTaHey-qeRt%2BnZTNJg%40mail.gmail.com
> >
> > --
> > Thanks,
> > Nisha
>
> Hi Nisha,
>
> Some comments.
>
> Patch 0001
> 1. Commit message
>
> from "two_phase enables slots." to "two_phase enabled slots"
>
> 2.
> +
> +     <para>
> +      When a subscription's
> +      <link linkend="sql-createsubscription-params-with-two-phase"><literal>two_phase</literal></link>
> +      is in the pending state, setting <literal>failover</literal> to
> true is not
> +      permitted. Once
>
> The term "subscription's two_phase" sounds a bit vague here.
> Referring https://www.postgresql.org/docs/17/sql-createsubscription.html
> and https://www.postgresql.org/docs/current/catalog-pg-subscription.html
> two_phase is referred to both as the "internal state" as well as the
> "mode" of the subscription.
>
> Probably rephrase it to - "When a subscription's internal two_phase
> mode is in the pending state, setting failover to true is not
> permitted. See column subtwophasestate of pg_subscription to know the
> actual internal state of the two-phase mode" (with appropriate tags
> and reference links)
>

There seems a potential muddling of terms two_phase VS two-phase.

AFAIK "two_phase" (with the underscore) is the name of the
subscription option; it might also be the name of an internal
structure member or catalog column name. Furthermore, when you are
referring to the option name I think it ought to be double-quoted for
clarity.

But, unless you are referring specifically to the option/field/column
then IMO it should say two-phase (with the dash).
e.g. The "two_phase" option enables two-phase mode.

======
Kind Regards,
Peter Smith.
Fujitsu Australia



Commits

  1. Don't retreat slot's confirmed_flush LSN.

  2. Fix assertion failure during decoding from synced slots.

  3. Fix xmin advancement during fast_forward decoding.

  4. Fix slot synchronization for two_phase enabled slots.

  5. Again match pg_user_mappings to information_schema.user_mapping_options.