public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bharath Rupireddy <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Disallow altering invalidated replication slots
Date: Tue, 10 Sep 2024 00:11:01 +0530
Message-ID: <CALj2ACW4fSOMiKjQ3=2NVBMTZRTG8Ujg6jsK9z3EvOtvA4vzKQ@mail.gmail.com> (raw)

Hi,

ALTER_REPLICATION_SLOT on invalidated replication slots is unnecessary
as there is no way to get the invalidated (logical) slot to work.
Please find the patch to add an error in such cases. Relevant
discussion is at [1].

Thoughts?

[1] https://www.postgresql.org/message-id/CAA4eK1%2Bszcosq0nS109mMSxPWyNT1Q%3DUNYCJgXKYuCceaPS%2BhA%40ma...

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/octet-stream] v1-0001-Disallow-altering-invalidated-replication-slots.patch (2.5K, ../CALj2ACW4fSOMiKjQ3=2NVBMTZRTG8Ujg6jsK9z3EvOtvA4vzKQ@mail.gmail.com/2-v1-0001-Disallow-altering-invalidated-replication-slots.patch)
  download | inline diff:
From 8b2a7e7c629c5d21fdb801739d9661722904dd7e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Mon, 9 Sep 2024 18:25:16 +0000
Subject: [PATCH v1] Disallow altering invalidated replication slots

ALTER_REPLICATION_SLOT on invalidated replication slots is
unnecessary as there is no way to get the invalidated
(logical) slot to work. This commit adds an error.

Discussion: https://www.postgresql.org/message-id/CAA4eK1%2Bszcosq0nS109mMSxPWyNT1Q%3DUNYCJgXKYuCceaPS%2BhA%40mail.gmail.com
---
 src/backend/replication/slot.c                      | 7 +++++++
 src/test/recovery/t/035_standby_logical_decoding.pl | 9 +++++++++
 2 files changed, 16 insertions(+)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 0a03776156..35652d9ad8 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -820,6 +820,13 @@ ReplicationSlotAlter(const char *name, const bool *failover,
 				errmsg("cannot use %s with a physical replication slot",
 					   "ALTER_REPLICATION_SLOT"));
 
+	if (MyReplicationSlot->data.invalidated != RS_INVAL_NONE)
+		ereport(ERROR,
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("cannot alter replication slot \"%s\"", name),
+				errdetail("This replication slot was invalidated due to \"%s\".",
+						  SlotInvalidationCauses[MyReplicationSlot->data.invalidated]));
+
 	if (RecoveryInProgress())
 	{
 		/*
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 4185b803b2..75ccfca409 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -526,6 +526,15 @@ check_for_invalidation('vacuum_full_', 1, 'with vacuum FULL on pg_class');
 # Verify reason for conflict is 'rows_removed' in pg_replication_slots
 check_slots_conflict_reason('vacuum_full_', 'rows_removed');
 
+# Attempting to alter an invalidated slot should result in an error
+($result, $stdout, $stderr) = $node_standby->psql(
+    'postgres',
+    qq[ALTER_REPLICATION_SLOT vacuum_full_inactiveslot (failover);],
+    replication => 'database');
+ok($stderr =~ /ERROR:  cannot alter replication slot "vacuum_full_inactiveslot"/ &&
+   $stderr =~ /DETAIL:  This replication slot was invalidated due to "rows_removed"./,
+    "invalidated slot cannot be altered");
+
 # Ensure that replication slot stats are not removed after invalidation.
 is( $node_standby->safe_psql(
 		'testdb',
-- 
2.43.0



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]
  Subject: Re: Disallow altering invalidated replication slots
  In-Reply-To: <CALj2ACW4fSOMiKjQ3=2NVBMTZRTG8Ujg6jsK9z3EvOtvA4vzKQ@mail.gmail.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