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