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 <[email protected]> wrote:
> > 
> > Hi
> > 
> > A failover option has been added to the CREATE SUBSCRITION command, but 
> > this functionality isn't easily accessible using the 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: enable syncing 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 “create subscription” 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/
> 
> 
> 
> 
>From 566378062516a5f2bc0a28fa6337996db56f7927 Mon Sep 17 00:00:00 2001
From: Ioseph Kim <[email protected]>
Date: Wed, 10 Dec 2025 17:26:02 +0900
Subject: [PATCH] Adding a '--enable-failover' option to 'pg_createsubscriber'
 utility

A failover option has been added to the CREATE SUBSCRITION command,
but this functionality isn't easily accessible using
the pg_createsubscriber tool.
Subscriptions created using pg_createsubscriber must be configured
for failover via an ALTER command.
To address this issue, I added a simple --enable-failover option
to the pg_createsubscriber tool.
---
 doc/src/sgml/ref/pg_createsubscriber.sgml   | 12 ++++++++++
 src/bin/pg_basebackup/pg_createsubscriber.c | 26 ++++++++++++++++-----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index bb9cc72576c..0a22d148fa3 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -272,6 +272,18 @@ PostgreSQL documentation
      </listitem>
     </varlistentry>
 
+    <varlistentry>
+     <term><option>--enable-failover</option></term>
+     <listitem>
+      <para>
+       Enables the subscription's
+       <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link>
+       option, allowing its logical replication slot to be used after failover.
+       The default is <literal>false</literal>.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry>
      <term><option>--publication=<replaceable class="parameter">name</replaceable></option></term>
      <listitem>
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index ef6deec14af..82506423f68 100644
--- 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,8 @@ 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 */
 };
@@ -260,6 +263,8 @@ usage(void)
 	printf(_("      --publication=NAME          publication name\n"));
 	printf(_("      --replication-slot=NAME     replication slot name\n"));
 	printf(_("      --subscription=NAME         subscription name\n"));
+	printf(_("      --enable-failover           enable syncing replication slots associated\n"
+			 "                                  with the subscription\n"));
 	printf(_("  -V, --version                   output version information, then exit\n"));
 	printf(_("  -?, --help                      show this help, then exit\n"));
 	printf(_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
@@ -507,16 +512,18 @@ store_pub_sub_info(const struct CreateSubscriberOptions *opt,
 			dbinfo[i].subname = subcell->val;
 		else
 			dbinfo[i].subname = NULL;
+		dbinfos.failover = opt->failover;
 		/* Other fields will be filled later */
 
 		pg_log_debug("publisher(%d): publication: %s ; replication slot: %s ; connection string: %s", i,
 					 dbinfo[i].pubname ? dbinfo[i].pubname : "(auto)",
 					 dbinfo[i].replslotname ? dbinfo[i].replslotname : "(auto)",
 					 dbinfo[i].pubconninfo);
-		pg_log_debug("subscriber(%d): subscription: %s ; connection string: %s, two_phase: %s", i,
+		pg_log_debug("subscriber(%d): subscription: %s ; connection string: %s, two_phase: %s, failover: %s", i,
 					 dbinfo[i].subname ? dbinfo[i].subname : "(auto)",
 					 dbinfo[i].subconninfo,
-					 dbinfos.two_phase ? "true" : "false");
+					 dbinfos.two_phase ? "true" : "false",
+					 dbinfos.failover ? "true" : "false");
 
 		if (num_pubs > 0)
 			pubcell = pubcell->next;
@@ -1395,9 +1402,10 @@ create_logical_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo)
 	slot_name_esc = PQescapeLiteral(conn, slot_name, strlen(slot_name));
 
 	appendPQExpBuffer(str,
-					  "SELECT lsn FROM pg_catalog.pg_create_logical_replication_slot(%s, 'pgoutput', false, %s, false)",
+					  "SELECT lsn FROM pg_catalog.pg_create_logical_replication_slot(%s, 'pgoutput', false, %s, %s)",
 					  slot_name_esc,
-					  dbinfos.two_phase ? "true" : "false");
+					  dbinfos.two_phase ? "true" : "false",
+					  dbinfos.failover ? "true" : "false");
 
 	PQfreemem(slot_name_esc);
 
@@ -1833,9 +1841,10 @@ create_subscription(PGconn *conn, const struct LogicalRepInfo *dbinfo)
 	appendPQExpBuffer(str,
 					  "CREATE SUBSCRIPTION %s CONNECTION %s PUBLICATION %s "
 					  "WITH (create_slot = false, enabled = false, "
-					  "slot_name = %s, copy_data = false, two_phase = %s)",
+					  "slot_name = %s, copy_data = false, two_phase = %s, failover = %s)",
 					  subname_esc, pubconninfo_esc, pubname_esc, replslotname_esc,
-					  dbinfos.two_phase ? "true" : "false");
+					  dbinfos.two_phase ? "true" : "false",
+					  dbinfos.failover ? "true" : "false");
 
 	PQfreemem(pubname_esc);
 	PQfreemem(subname_esc);
@@ -2079,6 +2088,7 @@ main(int argc, char **argv)
 		{"replication-slot", required_argument, NULL, 3},
 		{"subscription", required_argument, NULL, 4},
 		{"clean", required_argument, NULL, 5},
+		{"enable-failover", no_argument, NULL, 6},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -2127,6 +2137,7 @@ main(int argc, char **argv)
 	opt.sub_port = DEFAULT_SUB_PORT;
 	opt.sub_username = NULL;
 	opt.two_phase = false;
+	opt.failover = false;
 	opt.database_names = (SimpleStringList)
 	{
 		0
@@ -2232,6 +2243,9 @@ main(int argc, char **argv)
 				else
 					pg_fatal("object type \"%s\" specified more than once for --clean", optarg);
 				break;
+			case 6:
+				opt.failover = true;
+				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-- 
2.47.3

Reply via email to