Re: Adding a '--two-phase' option to 'pg_createsubscriber' utility.

From: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: Ajin Cherian <itsajin(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Adding a '--two-phase' option to 'pg_createsubscriber' utility.
Date: 2025-01-17 04:22:07
Message-ID: CAHv8RjKLsFDFqeFkeDuZybDmd5a3MpMGn6OD9mmWNUj9GOxNeQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 17, 2025 at 5:43 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Shubham,
>
> Some review comments for patch v11-0001.
>
> ======
> src/sgml/ref/pg_createsubscriber.sgml
>
> 1.
> - must accept local connections.
> + must accept local connections. If you are planning to use the
> + <option>--enable-two-phase</option> then you will also need to set the
> + <xref linkend="guc-max-prepared-transactions"/> appropriately.
> </para>
>
>
> The missing word "switch"?
>
> /If you are planning to use the <option>--enable-two-phase</option>/If
> you are planning to use the <option>--enable-two-phase</option>
> switch/
>
>
> 2.
> <para>
> + Unless the '--enable-two-phase' switch is specified,
> <application>pg_createsubscriber</application> sets up logical
>
> Remove the single quotes, and use the proper SGML markup same as other
> places where --enable-two-phase is mentioned.
>
> <option>--enable-two-phase</option>
>
> ======
>
>
> 3.
> + pg_log_warning_hint("You can use '--enable-two_phase' switch to
> enable two_phase.");
>
> 3a.
> Typo. The real switch name does not have underscores.
>
> /--enable-two_phase/--enable-two-phase/
>
> ~
>
> 3b.
> I checked other PG source code, but I couldn't find any examples where
> the switch is given in single quotes quite like this.
>
> Maybe choose from one of the below instead:
>
> SUGGESTION #1
> Use the \"--enable-two-phase\" switch to enable two_phase.
>
> SUGGESTION #2
> Use the --enable-two-phase switch to enable two_phase.
>

Fixed the given comments. The attached patch contains the required changes.

Thanks and regards,
Shubham Khanna.

Attachment Content-Type Size
v12-0001-Add-support-for-two-phase-commit-in-pg_createsub.patch application/octet-stream 14.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-01-17 05:00:49 Remove XLogRecGetFullXid() in xlogreader.c?
Previous Message Tom Lane 2025-01-17 03:41:12 Re: Automatic update of time column