Fix synchronized_standby_slots GUC check hook

The validate_sync_standby_slots subroutine requires an LWLock, so it
cannot run in processes without PGPROC; skip it there to avoid a crash.

This replaces the current test for ReplicationSlotCtl being not null,
which appears to be a solution for the same problem but less general.
I also rewrote a related comment that mentioned ReplicationSlotCtl in
StandbySlotsHaveCaughtup.

This code came in with commit bf279ddd1c28; backpatch to 17.

Reported-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Discussion: https://postgr.es/m/202411281216.sutbxtr6idnn@alvherre.pgsql

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 887e38d56ef93a9cb29fed324860b1e0ec94a5e0..4a206f9527791e45fbeea287963d639173acc741 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2456,18 +2456,15 @@ validate_sync_standby_slots(char *rawname, List **elemlist)
    {
        GUC_check_errdetail("List syntax is invalid.");
    }
-   else if (!ReplicationSlotCtl)
+   else if (MyProc)
    {
        /*
-        * We cannot validate the replication slot if the replication slots'
-        * data has not been initialized. This is ok as we will anyway
-        * validate the specified slot when waiting for them to catch up. See
-        * StandbySlotsHaveCaughtup() for details.
+        * Check that each specified slot exist and is physical.
+        *
+        * Because we need an LWLock, we cannot do this on processes without a
+        * PGPROC, so we skip it there; but see comments in
+        * StandbySlotsHaveCaughtup() as to why that's not a problem.
         */
-   }
-   else
-   {
-       /* Check that the specified slots exist and are logical slots */
        LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 
        foreach_ptr(char, name, *elemlist)
@@ -2649,18 +2646,18 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
 
        slot = SearchNamedReplicationSlot(name, false);
 
+       /*
+        * If a slot name provided in synchronized_standby_slots does not
+        * exist, report a message and exit the loop.
+        *
+        * Though validate_sync_standby_slots (the GUC check_hook) tries to
+        * avoid this, it can nonetheless happen because the user can specify
+        * a nonexistent slot name before server startup. That function cannot
+        * validate such a slot during startup, as ReplicationSlotCtl is not
+        * initialized by then.  Also, the user might have dropped one slot.
+        */
        if (!slot)
        {
-           /*
-            * If a slot name provided in synchronized_standby_slots does not
-            * exist, report a message and exit the loop. A user can specify a
-            * slot name that does not exist just before the server startup.
-            * The GUC check_hook(validate_sync_standby_slots) cannot validate
-            * such a slot during startup as the ReplicationSlotCtl shared
-            * memory is not initialized at that time. It is also possible for
-            * a user to drop the slot in synchronized_standby_slots
-            * afterwards.
-            */
            ereport(elevel,
                    errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                    errmsg("replication slot \"%s\" specified in parameter \"%s\" does not exist",
@@ -2672,16 +2669,9 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
            break;
        }
 
+       /* Same as above: if a slot is not physical, exit the loop. */
        if (SlotIsLogical(slot))
        {
-           /*
-            * If a logical slot name is provided in
-            * synchronized_standby_slots, report a message and exit the loop.
-            * Similar to the non-existent case, a user can specify a logical
-            * slot name in synchronized_standby_slots before the server
-            * startup, or drop an existing physical slot and recreate a
-            * logical slot with the same name.
-            */
            ereport(elevel,
                    errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                    errmsg("cannot specify logical replication slot \"%s\" in parameter \"%s\"",