Thread
-
Propose: Adding a '--enable-failover' option to 'pg_createsubscriber'
Ioseph Kim <pgsql-kr@postgresql.kr> — 2025-12-10T09:03:48Z
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
-
Re:Propose: Adding a '--enable-failover' option to 'pg_createsubscriber'
Xueyu Gao <gaoxueyu_hope@163.com> — 2025-12-10T10:11:46Z
Hi, I have one comment to following comment: pg_createsubscriber.c, the comment is not correct as this new option is not related with logical replication slot. bool failover; /* enable failover option of logical replication slot */ Suggest to change to: /* enable failover option of subscription */ Thanks, Xueyu Gao HighGo Software Co., Ltd. https://www.highgo.com/ At 2025-12-10 17:03:48, "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 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
-
Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber'
Chao Li <li.evan.chao@gmail.com> — 2025-12-10T23:28:13Z
> 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 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/ -
Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber'
Ioseph Kim <pgsql-kr@postgresql.kr> — 2025-12-11T01:59:49Z
thanks for your comment. There are two failover variables, one is CreateSubscriberOptions.failover, the other is LogicalRepInfos.failover. That comment is for LogicalRepInfos.failover. LogicalRepInfos.failover variable is used when logical replication slot will be created for the subscription. ioseph On Wed, Dec 10, 2025 at 06:11:46PM +0800, 高雪玉 wrote: > Hi, > I have one comment to following comment: > > > pg_createsubscriber.c, the comment is not correct as this new option is not related with logical replication slot. > bool failover; /* enable failover option of logical replication slot */ > > > Suggest to change to: > /* enable failover option of subscription */ > > > Thanks, > Xueyu Gao > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > At 2025-12-10 17:03:48, "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 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
-
Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber'
Ioseph Kim <pgsql-kr@postgresql.kr> — 2025-12-11T04:02:24Z
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 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/ > > > > -
Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber' ver.3
Ioseph Kim <pgsql-kr@postgresql.kr> — 2025-12-11T06:39:16Z
Hi TAP test codes are added in the patch and manual has become more user-friendly. Please review the patch and let me know if you see a better solution. I’ll be glad to update it accordingly. ioseph On Wed, Dec 10, 2025 at 06:03:48PM +0900, Ioseph Kim 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
-
Re:Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber' ver.3
Xueyu Gao <gaoxueyu_hope@163.com> — 2025-12-11T07:13:40Z
At 2025-12-11 14:39:16, "Ioseph Kim" <pgsql-kr@postgresql.kr> wrote: >Hi > >TAP test codes are added in the patch >and manual has become more user-friendly. > >Please review the patch and let me know if you see a better solution. I’ll be glad to update it accordingly. > >ioseph > >On Wed, Dec 10, 2025 at 06:03:48PM +0900, Ioseph Kim 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 Hi, ioseph, I took a look at the doc part and have two comments. /doc/src/sgml/ref/pg_createsubscriber.sgml 1. "The default is <literal>false</literal>" , it'd be better to add "value" after the "default". 2. I think the following content should be surrounded by <para> and </para>, so the <para> should be moved to be in front of "When this option is enabled..." + When this option is enabled, the connection string used in the <option>--publisher-server</option> + option may be adjusted to support failover. For example, by specifying multiple hosts + and using <literal>target_session_attrs=read-write</literal>. + <para> + </para> + </listitem> + </varlistentry> Thanks, Xueyu Gao
-
Re:Re:Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber' ver.3
Xueyu Gao <gaoxueyu_hope@163.com> — 2025-12-11T07:51:53Z
At 2025-12-11 15:13:40, "Xueyu Gao" <gaoxueyu_hope@163.com> wrote: At 2025-12-11 14:39:16, "Ioseph Kim" <pgsql-kr@postgresql.kr> wrote: >Hi > >TAP test codes are added in the patch >and manual has become more user-friendly. > >Please review the patch and let me know if you see a better solution. I’ll be glad to update it accordingly. > >ioseph > >On Wed, Dec 10, 2025 at 06:03:48PM +0900, Ioseph Kim 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 Hi, ioseph, I took a look at the doc part and have two comments. /doc/src/sgml/ref/pg_createsubscriber.sgml 1. "The default is <literal>false</literal>" , it'd be better to add "value" after the "default". 2. I think the following content should be surrounded by <para> and </para>, so the <para> should be moved to be in front of "When this option is enabled..." + When this option is enabled, the connection string used in the <option>--publisher-server</option> + option may be adjusted to support failover. For example, by specifying multiple hosts + and using <literal>target_session_attrs=read-write</literal>. + <para> + </para> + </listitem> + </varlistentry> Thanks, Xueyu Gao One more comment: src/bin/pg_basebackup/t/040_pg_createsubscriber.pl + 'replication slot are created with the failover option enabled'); This sentence should be "replication slots are ..." Thanks, Xueyu Gao
-
Re: Re: Propose: Adding a '--enable-failover' option to 'pg_createsubscriber' ver.3
Ioseph Kim <pgsql-kr@postgresql.kr> — 2025-12-11T08:03:01Z
Thanks Xueyu 1. In this manual, The phrase "The default is ..." is used consistently. so, It would be better not to change it to maintain consistency. 2. I made a mistake. If this gets committed, a contributer would fix the <para> line to be placed at the beginning of the paragraph, please. ioseph On Thu, Dec 11, 2025 at 03:13:40PM +0800, Xueyu Gao wrote: > > > At 2025-12-11 14:39:16, "Ioseph Kim" <pgsql-kr@postgresql.kr> wrote: > >Hi > > > >TAP test codes are added in the patch > >and manual has become more user-friendly. > > > >Please review the patch and let me know if you see a better solution. I’ll be glad to update it accordingly. > > > >ioseph > > > >On Wed, Dec 10, 2025 at 06:03:48PM +0900, Ioseph Kim 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 > > > Hi, ioseph, > > I took a look at the doc part and have two comments. > > /doc/src/sgml/ref/pg_createsubscriber.sgml > > 1. "The default is <literal>false</literal>" , it'd be better to add "value" after the "default". > > 2. I think the following content should be surrounded by <para> and </para>, so the <para> should be moved to be in front of "When this option is enabled..." > > + When this option is enabled, the connection string used in the <option>--publisher-server</option> > > + option may be adjusted to support failover. For example, by specifying multiple hosts > > + and using <literal>target_session_attrs=read-write</literal>. > > + <para> > > + </para> > > + </listitem> > > + </varlistentry> > Thanks, > Xueyu Gao