From b29cbd3da4e37db17026b9fe58fb46fe83f467bf Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Tue, 23 Apr 2024 12:14:57 +0530 Subject: Fix the handling of the failover option in subscription commands. Do not allow ALTER SUBSCRIPTION ... SET (failover = on|off) in a transaction block as the changed failover option of the slot can't be rolled back. For the same reason, we refrain from altering the replication slot's failover property if the subscription is created with a valid slot_name and create_slot=false. Reprted-by: Kuroda Hayato Author: Hou Zhijie Reviewed-by: Shveta Malik, Bertrand Drouvot, Kuroda Hayato Discussion: https://postgr.es/m/OS0PR01MB57165542B09DFA4943830BF294082@OS0PR01MB5716.jpnprd01.prod.outlook.com --- src/backend/commands/subscriptioncmds.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) (limited to 'src/backend/commands') diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 5a47fa984df..e407428dbcf 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -401,13 +401,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, errmsg("%s and %s are mutually exclusive options", "connect = false", "copy_data = true"))); - if (opts->failover && - IsSet(opts->specified_opts, SUBOPT_FAILOVER)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("%s and %s are mutually exclusive options", - "connect = false", "failover = true"))); - /* Change the defaults of other options. */ opts->enabled = false; opts->create_slot = false; @@ -836,21 +829,6 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, (errmsg("created replication slot \"%s\" on publisher", opts.slot_name))); } - - /* - * If the slot_name is specified without the create_slot option, - * it is possible that the user intends to use an existing slot on - * the publisher, so here we alter the failover property of the - * slot to match the failover value in subscription. - * - * We do not need to change the failover to false if the server - * does not support failover (e.g. pre-PG17). - */ - else if (opts.slot_name && - (opts.failover || walrcv_server_version(wrconn) >= 170000)) - { - walrcv_alter_slot(wrconn, opts.slot_name, opts.failover); - } } PG_FINALLY(); { @@ -1267,6 +1245,12 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, errmsg("cannot set %s for enabled subscription", "failover"))); + /* + * The changed failover option of the slot can't be rolled + * back. + */ + PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION ... SET (failover)"); + values[Anum_pg_subscription_subfailover - 1] = BoolGetDatum(opts.failover); replaces[Anum_pg_subscription_subfailover - 1] = true; -- cgit v1.2.3