public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bharath Rupireddy <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Add lookup table for replication slot invalidation causes
Date: Tue, 20 Feb 2024 16:40:44 +0530
Message-ID: <CALj2ACUxSLA91QGFrJsWNKs58KXb1C03mbuwKmzqqmoAKLwJaw@mail.gmail.com> (raw)

Hi,

Presently, replication slot invalidation causes and their text are
scattered into ReplicationSlotInvalidationCause enum and a bunch of
macros. This is making the code to get invalidation cause text given
the cause as enum and vice-versa unreadable, longer and inextensible.
The attached patch adds a lookup table for all invalidation causes for
better readability and extensibility. FWIW, another patch in
discussion https://www.postgresql.org/message-id/[email protected]...
adds a couple of other invalidation reasons, this lookup table makes
the life easier and code shorter.

Thoughts?

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


Attachments:

  [application/octet-stream] v1-0001-Add-lookup-table-for-replication-slot-invalidatio.patch (4.8K, ../CALj2ACUxSLA91QGFrJsWNKs58KXb1C03mbuwKmzqqmoAKLwJaw@mail.gmail.com/2-v1-0001-Add-lookup-table-for-replication-slot-invalidatio.patch)
  download | inline diff:
From b92b516d0face659460766bb23b7e8a8e367d5e0 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Tue, 20 Feb 2024 11:06:20 +0000
Subject: [PATCH v1] Add lookup table for replication slot invalidation causes

---
 src/backend/replication/slot.c      | 36 +++++++++++++++++------------
 src/backend/replication/slotfuncs.c | 24 +++++--------------
 src/include/replication/slot.h      | 16 ++++++-------
 3 files changed, 35 insertions(+), 41 deletions(-)

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index a142855bd3..aff62f2eb4 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -77,6 +77,19 @@ typedef struct ReplicationSlotOnDisk
 	ReplicationSlotPersistentData slotdata;
 } ReplicationSlotOnDisk;
 
+/*
+ * Lookup table for invalidation cause names indexed by RS_INVAL_XXX.
+ */
+const char *const SlotInvalidationCauses[] = {
+	"none",						/* RS_INVAL_NONE */
+	"wal_removed",				/* RS_INVAL_WAL_REMOVED */
+	"rows_removed",				/* RS_INVAL_HORIZON */
+	"wal_level_insufficient"	/* RS_INVAL_WAL_LEVEL */
+};
+
+StaticAssertDecl(lengthof(SlotInvalidationCauses) == (RS_INVAL_MAX_CAUSES + 1),
+				 "array length mismatch");
+
 /* size of version independent data */
 #define ReplicationSlotOnDiskConstantSize \
 	offsetof(ReplicationSlotOnDisk, slotdata)
@@ -2290,23 +2303,16 @@ RestoreSlotFromDisk(const char *name)
 }
 
 /*
- * Maps the pg_replication_slots.conflict_reason text value to
- * ReplicationSlotInvalidationCause enum value
+ * Look up invalidation cause by name.
  */
 ReplicationSlotInvalidationCause
-GetSlotInvalidationCause(char *conflict_reason)
+GetSlotInvalidationCause(char *name)
 {
-	Assert(conflict_reason);
-
-	if (strcmp(conflict_reason, SLOT_INVAL_WAL_REMOVED_TEXT) == 0)
-		return RS_INVAL_WAL_REMOVED;
-	else if (strcmp(conflict_reason, SLOT_INVAL_HORIZON_TEXT) == 0)
-		return RS_INVAL_HORIZON;
-	else if (strcmp(conflict_reason, SLOT_INVAL_WAL_LEVEL_TEXT) == 0)
-		return RS_INVAL_WAL_LEVEL;
-	else
-		Assert(0);
+	ReplicationSlotInvalidationCause cause = RS_INVAL_NONE;
+
+	for (cause = RS_INVAL_NONE; cause <= RS_INVAL_MAX_CAUSES; cause++)
+		if (strcmp(SlotInvalidationCauses[cause], name) == 0)
+			break;
 
-	/* Keep compiler quiet */
-	return RS_INVAL_NONE;
+	return cause;
 }
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index d2fa5e669a..c108bf9608 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -413,24 +413,12 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 			nulls[i++] = true;
 		else
 		{
-			switch (slot_contents.data.invalidated)
-			{
-				case RS_INVAL_NONE:
-					nulls[i++] = true;
-					break;
-
-				case RS_INVAL_WAL_REMOVED:
-					values[i++] = CStringGetTextDatum(SLOT_INVAL_WAL_REMOVED_TEXT);
-					break;
-
-				case RS_INVAL_HORIZON:
-					values[i++] = CStringGetTextDatum(SLOT_INVAL_HORIZON_TEXT);
-					break;
-
-				case RS_INVAL_WAL_LEVEL:
-					values[i++] = CStringGetTextDatum(SLOT_INVAL_WAL_LEVEL_TEXT);
-					break;
-			}
+			ReplicationSlotInvalidationCause cause = slot_contents.data.invalidated;
+
+			if (cause == RS_INVAL_NONE)
+				nulls[i++] = true;
+			else
+				values[i++] = CStringGetTextDatum(SlotInvalidationCauses[cause]);
 		}
 
 		values[i++] = BoolGetDatum(slot_contents.data.failover);
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index e706ca834c..b26697ba15 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -40,6 +40,9 @@ typedef enum ReplicationSlotPersistency
 /*
  * Slots can be invalidated, e.g. due to max_slot_wal_keep_size. If so, the
  * 'invalidated' field is set to a value other than _NONE.
+ *
+ * If you add a new invalidation cause here, remember to add its name in
+ * SlotInvalidationCauses in the same order as that of the cause.
  */
 typedef enum ReplicationSlotInvalidationCause
 {
@@ -52,13 +55,10 @@ typedef enum ReplicationSlotInvalidationCause
 	RS_INVAL_WAL_LEVEL,
 } ReplicationSlotInvalidationCause;
 
-/*
- * The possible values for 'conflict_reason' returned in
- * pg_get_replication_slots.
- */
-#define SLOT_INVAL_WAL_REMOVED_TEXT "wal_removed"
-#define SLOT_INVAL_HORIZON_TEXT     "rows_removed"
-#define SLOT_INVAL_WAL_LEVEL_TEXT   "wal_level_insufficient"
+/* Maximum number of invalidation causes */
+#define	RS_INVAL_MAX_CAUSES RS_INVAL_WAL_LEVEL
+
+extern PGDLLIMPORT const char *const SlotInvalidationCauses[];
 
 /*
  * On-Disk data of a replication slot, preserved across restarts.
@@ -275,6 +275,6 @@ extern void CheckPointReplicationSlots(bool is_shutdown);
 extern void CheckSlotRequirements(void);
 extern void CheckSlotPermissions(void);
 extern ReplicationSlotInvalidationCause
-			GetSlotInvalidationCause(char *conflict_reason);
+			GetSlotInvalidationCause(char *cause);
 
 #endif							/* SLOT_H */
-- 
2.34.1



view thread (2+ messages)

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: Add lookup table for replication slot invalidation causes
  In-Reply-To: <CALj2ACUxSLA91QGFrJsWNKs58KXb1C03mbuwKmzqqmoAKLwJaw@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