public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zhijie Hou (Fujitsu) <[email protected]>
To: pgsql-hackers <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: Disallow changing slot's failover option in transaction block
Date: Tue, 16 Apr 2024 06:32:11 +0000
Message-ID: <OS0PR01MB57165542B09DFA4943830BF294082@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
Hi,
Kuroda-San reported an issue off-list that:
If user execute ALTER SUBSCRIPTION SET (failover) command inside a txn block
and rollback, only the subscription option change can be rolled back, while the
replication slot's failover change is preserved.
This is because ALTER SUBSCRIPTION SET (failover) command internally executes
the replication command ALTER_REPLICATION_SLOT to change the replication slot's
failover property, but this replication command execution cannot be
rollback.
To fix it, I think we can prevent user from executing ALTER SUBSCRIPTION set
(failover) inside a txn block, which is also consistent to the ALTER
SUBSCRIPTION REFRESH/DROP SUBSCRIPTION command. Attach a small
patch to address this.
Best Regards,
Hou Zhijie
Attachments:
[application/octet-stream] v1-0001-Disallow-alter-subscription-s-failover-option-ins.patch (3.0K, ../OS0PR01MB57165542B09DFA4943830BF294082@OS0PR01MB5716.jpnprd01.prod.outlook.com/2-v1-0001-Disallow-alter-subscription-s-failover-option-ins.patch)
download | inline diff:
From d792ab2fa2aefb8842774aa44478eebd35851abf Mon Sep 17 00:00:00 2001
From: Hou Zhijie <[email protected]>
Date: Tue, 16 Apr 2024 11:31:32 +0800
Subject: [PATCH v1] Disallow alter subscription's failover option inside a
transaction block
Disallow ALTER SUBSCRIPTION in a transaction block when the replication slot is
to be altered, since that cannot be rolled back.
---
src/backend/commands/subscriptioncmds.c | 9 +++++++++
src/test/regress/expected/subscription.out | 5 +++++
src/test/regress/sql/subscription.sql | 5 +++++
3 files changed, 19 insertions(+)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 5a47fa984d..9fa5f4e5b4 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1267,6 +1267,15 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
errmsg("cannot set %s for enabled subscription",
"failover")));
+ /*
+ * Since altering a replication slot is not transactional,
+ * rolling back the transaction leaves the altered
+ * replication slot. So we cannot run ALTER SUBSCRIPTION
+ * inside a transaction block if altering a replication
+ * slot's failover option.
+ */
+ 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/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 1eee6b17b8..0e6c408be3 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -472,6 +472,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 1b2a23ba7b..07413d7e81 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -333,6 +333,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;
--
2.31.1
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Disallow changing slot's failover option in transaction block
In-Reply-To: <OS0PR01MB57165542B09DFA4943830BF294082@OS0PR01MB5716.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox