diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 413ce68ce2c..a78c1c3a478 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -66,10 +66,11 @@ ALTER SUBSCRIPTION name RENAME TO < - Commands ALTER SUBSCRIPTION ... REFRESH PUBLICATION and + Commands ALTER SUBSCRIPTION ... REFRESH PUBLICATION, ALTER SUBSCRIPTION ... {SET|ADD|DROP} PUBLICATION ... - with refresh option as true cannot be - executed inside a transaction block. + with refresh option as true and + ALTER SUBSCRIPTION ... SET (failover = on|off) + cannot be executed inside a transaction block. These commands also cannot be executed when the subscription has two_phase diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index 15794731bbb..740b7d94210 100644 --- a/doc/src/sgml/ref/create_subscription.sgml +++ b/doc/src/sgml/ref/create_subscription.sgml @@ -122,8 +122,7 @@ CREATE SUBSCRIPTION subscription_nameconnect to false with setting create_slot, enabled, - copy_data, or failover to - true.) + or copy_data to true.) @@ -183,6 +182,21 @@ CREATE SUBSCRIPTION subscription_name for examples. + + + When setting slot_name to a valid name and + create_slot to false, the + failover property value of the named slot may + differ from the counterpart failover parameter + specified in the subscription. Always ensure the slot property + failover matches the counterpart parameter of the + subscription and vice versa. Otherwise, the slot on the publisher may + behave differently from what these subscription options say: for + example, the slot on the publisher could either be synced to the + standbys even when the subscription's failover + option is disabled or could be disabled for sync even when the + subscription's failover option is enabled. + diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index b99793e4148..671df4b60ef 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -1611,11 +1611,7 @@ CREATE DATABASE foo WITH TEMPLATE template0; dump can be restored without requiring network access to the remote servers. It is then up to the user to reactivate the subscriptions in a suitable way. If the involved hosts have changed, the connection - information might have to be changed. If the subscription needs to - be enabled for - failover, - execute ALTER SUBSCRIPTION ... SET (failover = true) - after the slot has been created. It might also be appropriate to + information might have to be changed. It might also be appropriate to truncate the target tables before initiating a new full table copy. If users intend to copy initial data during refresh they must create the slot with two_phase = false. After the initial sync, the 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; diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index ed9bab3bfe5..b8acdd73551 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -4804,12 +4804,17 @@ getSubscriptions(Archive *fout) if (dopt->binary_upgrade && fout->remoteVersion >= 170000) appendPQExpBufferStr(query, " o.remote_lsn AS suboriginremotelsn,\n" - " s.subenabled,\n" - " s.subfailover\n"); + " s.subenabled,\n"); else appendPQExpBufferStr(query, " NULL AS suboriginremotelsn,\n" - " false AS subenabled,\n" - " false AS subfailover\n"); + " false AS subenabled,\n"); + + if (fout->remoteVersion >= 170000) + appendPQExpBufferStr(query, + " s.subfailover\n"); + else + appendPQExpBuffer(query, + " false AS subfailover\n"); appendPQExpBufferStr(query, "FROM pg_subscription s\n"); @@ -5132,6 +5137,9 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo) if (strcmp(subinfo->subrunasowner, "t") == 0) appendPQExpBufferStr(query, ", run_as_owner = true"); + if (strcmp(subinfo->subfailover, "t") == 0) + appendPQExpBufferStr(query, ", failover = true"); + if (strcmp(subinfo->subsynccommit, "off") != 0) appendPQExpBuffer(query, ", synchronous_commit = %s", fmtId(subinfo->subsynccommit)); @@ -5165,17 +5173,6 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo) appendPQExpBuffer(query, ", '%s');\n", subinfo->suboriginremotelsn); } - if (strcmp(subinfo->subfailover, "t") == 0) - { - /* - * Enable the failover to allow the subscription's slot to be - * synced to the standbys after the upgrade. - */ - appendPQExpBufferStr(query, - "\n-- For binary upgrade, must preserve the subscriber's failover option.\n"); - appendPQExpBuffer(query, "ALTER SUBSCRIPTION %s SET(failover = true);\n", qsubname); - } - if (strcmp(subinfo->subenabled, "t") == 0) { /* diff --git a/src/test/recovery/t/040_standby_failover_slots_sync.pl b/src/test/recovery/t/040_standby_failover_slots_sync.pl index 76545e3c74a..12acf874d70 100644 --- a/src/test/recovery/t/040_standby_failover_slots_sync.pl +++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl @@ -42,26 +42,12 @@ my $slot_creation_time_on_primary = $publisher->safe_psql( SELECT current_timestamp; ]); -# Create a slot on the publisher with failover disabled -$publisher->safe_psql('postgres', - "SELECT 'init' FROM pg_create_logical_replication_slot('lsub1_slot', 'pgoutput', false, false, false);" -); - -# Confirm that the failover flag on the slot is turned off -is( $publisher->safe_psql( - 'postgres', - q{SELECT failover from pg_replication_slots WHERE slot_name = 'lsub1_slot';} - ), - "f", - 'logical slot has failover false on the publisher'); - -# Create a subscription (using the same slot created above) that enables -# failover. +# Create a subscription that enables failover. $subscriber1->safe_psql('postgres', - "CREATE SUBSCRIPTION regress_mysub1 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (slot_name = lsub1_slot, copy_data=false, failover = true, create_slot = false, enabled = false);" + "CREATE SUBSCRIPTION regress_mysub1 CONNECTION '$publisher_connstr' PUBLICATION regress_mypub WITH (slot_name = lsub1_slot, copy_data = false, failover = true, enabled = false);" ); -# Confirm that the failover flag on the slot has now been turned on +# Confirm that the failover flag on the slot is turned on is( $publisher->safe_psql( 'postgres', q{SELECT failover from pg_replication_slots WHERE slot_name = 'lsub1_slot';} diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 1eee6b17b8f..0f2a25cdc19 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -89,8 +89,6 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU ERROR: connect = false and enabled = true are mutually exclusive options CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, create_slot = true); ERROR: connect = false and create_slot = true are mutually exclusive options -CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, failover = true); -ERROR: connect = false and failover = true are mutually exclusive options CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true); ERROR: slot_name = NONE and enabled = true are mutually exclusive options CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false, create_slot = true); @@ -472,6 +470,11 @@ REVOKE CREATE ON DATABASE REGRESSION FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; ALTER SUBSCRIPTION regress_testsub RENAME TO regress_testsub2; ERROR: permission denied for database regression +-- fail - cannot do ALTER SUBSCRIPTION SET (failover) inside transaction block +BEGIN; +ALTER SUBSCRIPTION regress_testsub SET (failover); +ERROR: ALTER SUBSCRIPTION ... SET (failover) cannot run inside a transaction block +COMMIT; -- ok, owning it is enough for this stuff ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE); DROP SUBSCRIPTION regress_testsub; diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 1b2a23ba7bf..3e5ba4cb8c6 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -54,7 +54,6 @@ SET SESSION AUTHORIZATION 'regress_subscription_user'; CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, copy_data = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, enabled = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, create_slot = true); -CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, failover = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false, create_slot = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE); @@ -333,6 +332,11 @@ REVOKE CREATE ON DATABASE REGRESSION FROM regress_subscription_user3; SET SESSION AUTHORIZATION regress_subscription_user3; ALTER SUBSCRIPTION regress_testsub RENAME TO regress_testsub2; +-- fail - cannot do ALTER SUBSCRIPTION SET (failover) inside transaction block +BEGIN; +ALTER SUBSCRIPTION regress_testsub SET (failover); +COMMIT; + -- ok, owning it is enough for this stuff ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE); DROP SUBSCRIPTION regress_testsub;