Hi, Evan
thanks for your comments.
I modified like a attached patch file.
I used pgindent, thank you.
CreateSubscriberOptions.failover is used in CREATE SUBSCRIPTION,
but LogicalRepInfos.failover is used in pg_create_logical_replication_slot()
so I left these comments as is.
ioseph
On Thu, Dec 11, 2025 at 07:28:13AM +0800, Chao Li wrote:
>
> > On Dec 10, 2025, at 17:03, Ioseph Kim <pgsql-kr@postgresql.kr> wrote:
> >
> > Hi
> >
> > A failover option has been added to the CREATE SUBSCRITION command, but this functionality isn't easily accessible
usingthe pg_createsubscriber tool.
> >
> > Subscriptions created using pg_createsubscriber must be configured for failover via an alter operation.
> >
> > To address this issue, we've added a simple --enable-failover option to the pg_createsubscriber tool.
> >
> > This patch is simple. It doesn't handle exceptions or provide any TAP test code.
> >
> > Please review this and we hope the development team will refine it further.
> >
> > ioseph
> > <0001-Adding-a-enable-failover-option-to-pg_createsubscrib.patch>
>
> Hi, Ioseph,
>
> Thanks for the patch. I consider adding the “(failover=true)” option is useful. A few small comments:
>
> 1
> ```
> + <para>
> + Enables <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
> + commit for the subscription.
> + The default is <literal>false</literal>.
> + </para>
> ```
>
> This doc change is confusing. What is “failover commit”? I guess you meant “failover option”?
>
> I’d suggest a phrase like:
>
> “Enables the subscription’s failover option, allowing its logical replication slot to be used after failover.”
>
> 2
> ```
> + printf(_(" --enable-failover enable failover\n"));
> ```
>
> If we look at the existing “—enable-two-phase” help text, we consider this help text is too simple. I’d suggest:
enablesyncing replication slots associated with the subscription.
>
> 3
> ```
> --- a/src/bin/pg_basebackup/pg_createsubscriber.c
> +++ b/src/bin/pg_basebackup/pg_createsubscriber.c
> @@ -49,6 +49,7 @@ struct CreateSubscriberOptions
> int recovery_timeout; /* stop recovery after this time */
> bool all_dbs; /* all option */
> SimpleStringList objecttypes_to_clean; /* list of object types to cleanup */
> + bool failover; /* enable failover option of subscription */
> };
>
> /* per-database publication/subscription info */
> @@ -73,6 +74,7 @@ struct LogicalRepInfos
> {
> struct LogicalRepInfo *dbinfo;
> bool two_phase; /* enable-two-phase option */
> + bool failover; /* enable failover option of logical replication slot */
> bits32 objecttypes_to_clean; /* flags indicating which object types
> * to clean up on subscriber */
> };
> ```
>
> Add comments for “failover” in the two structures are inconsistent. The latter one is incorrect, the option is for
“createsubscription” command but for a slot.
>
> 4 You need to run pgindent as I saw some format problems.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
