Make invalid primary_slot_name follow standard GUC error reporting.

Previously, if primary_slot_name was set to an invalid slot name and
the configuration file was reloaded, both the postmaster and all other
backend processes reported a WARNING. With many processes running,
this could produce a flood of duplicate messages. The problem was that
the GUC check hook for primary_slot_name reported errors at WARNING
level via ereport().

This commit changes the check hook to use GUC_check_errdetail() and
GUC_check_errhint() for error reporting. As with other GUC parameters,
this causes non-postmaster processes to log the message at DEBUG3,
so by default, only the postmaster's message appears in the log file.

Backpatch to all supported versions.

Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Chao Li <lic@highgo.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/CAHGQGwFud-cvthCTfusBfKHBS6Jj6kdAPTdLWKvP2qjUX6L_wA@mail.gmail.com
Backpatch-through: 13

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 52ff4d119e6f9369cf1cca1eed80af2c2b4bedc2..3e3c4da01a24cf7ade15bced3df8fb6bdc85f2ee 100644 (file)
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -4761,9 +4761,20 @@ RecoveryRequiresIntParameter(const char *param_name, int currValue, int minValue
 bool
 check_primary_slot_name(char **newval, void **extra, GucSource source)
 {
+       int                     err_code;
+       char       *err_msg = NULL;
+       char       *err_hint = NULL;
+
        if (*newval && strcmp(*newval, "") != 0 &&
-               !ReplicationSlotValidateName(*newval, false, WARNING))
+               !ReplicationSlotValidateNameInternal(*newval, false, &err_code,
+                                                                                        &err_msg, &err_hint))
+       {
+               GUC_check_errcode(err_code);
+               GUC_check_errdetail("%s", err_msg);
+               if (err_hint != NULL)
+                       GUC_check_errhint("%s", err_hint);
                return false;
+       }
 
        return true;
 }

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index ac188bb2f77156a18e4255e96cbde7d44651d259..a4ca363f20da8794d3362bbadddc9a878dc577f3 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -260,35 +260,72 @@ ReplicationSlotShmemExit(int code, Datum arg)
 /*
  * Check whether the passed slot name is valid and report errors at elevel.
  *
+ * See comments for ReplicationSlotValidateNameInternal().
+ */
+bool
+ReplicationSlotValidateName(const char *name, bool allow_reserved_name,
+                                                       int elevel)
+{
+       int                     err_code;
+       char       *err_msg = NULL;
+       char       *err_hint = NULL;
+
+       if (!ReplicationSlotValidateNameInternal(name, allow_reserved_name,
+                                                                                        &err_code, &err_msg, &err_hint))
+       {
+               /*
+                * Use errmsg_internal() and errhint_internal() instead of errmsg()
+                * and errhint(), since the messages from
+                * ReplicationSlotValidateNameInternal() are already translated. This
+                * avoids double translation.
+                */
+               ereport(elevel,
+                               errcode(err_code),
+                               errmsg_internal("%s", err_msg),
+                               (err_hint != NULL) ? errhint_internal("%s", err_hint) : 0);
+
+               pfree(err_msg);
+               if (err_hint != NULL)
+                       pfree(err_hint);
+               return false;
+       }
+
+       return true;
+}
+
+/*
+ * Check whether the passed slot name is valid.
+ *
  * An error will be reported for a reserved replication slot name if
  * allow_reserved_name is set to false.
  *
  * Slot names may consist out of [a-z0-9_]{1,NAMEDATALEN-1} which should allow
  * the name to be used as a directory name on every supported OS.
  *
- * Returns whether the directory name is valid or not if elevel < ERROR.
+ * Returns true if the slot name is valid. Otherwise, returns false and stores
+ * the error code, error message, and optional hint in err_code, err_msg, and
+ * err_hint, respectively. The caller is responsible for freeing err_msg and
+ * err_hint, which are palloc'd.
  */
 bool
-ReplicationSlotValidateName(const char *name, bool allow_reserved_name,
-                                                       int elevel)
+ReplicationSlotValidateNameInternal(const char *name, bool allow_reserved_name,
+                                                                       int *err_code, char **err_msg, char **err_hint)
 {
        const char *cp;
 
        if (strlen(name) == 0)
        {
-               ereport(elevel,
-                               (errcode(ERRCODE_INVALID_NAME),
-                                errmsg("replication slot name \"%s\" is too short",
-                                               name)));
+               *err_code = ERRCODE_INVALID_NAME;
+               *err_msg = psprintf(_("replication slot name \"%s\" is too short"), name);
+               *err_hint = NULL;
                return false;
        }
 
        if (strlen(name) >= NAMEDATALEN)
        {
-               ereport(elevel,
-                               (errcode(ERRCODE_NAME_TOO_LONG),
-                                errmsg("replication slot name \"%s\" is too long",
-                                               name)));
+               *err_code = ERRCODE_NAME_TOO_LONG;
+               *err_msg = psprintf(_("replication slot name \"%s\" is too long"), name);
+               *err_hint = NULL;
                return false;
        }
 
@@ -298,24 +335,19 @@ ReplicationSlotValidateName(const char *name, bool allow_reserved_name,
                          || (*cp >= '0' && *cp <= '9')
                          || (*cp == '_')))
                {
-                       ereport(elevel,
-                                       (errcode(ERRCODE_INVALID_NAME),
-                                        errmsg("replication slot name \"%s\" contains invalid character",
-                                                       name),
-                                        errhint("Replication slot names may only contain lower case letters, numbers, and the underscore character.")));
+                       *err_code = ERRCODE_INVALID_NAME;
+                       *err_msg = psprintf(_("replication slot name \"%s\" contains invalid character"), name);
+                       *err_hint = psprintf(_("Replication slot names may only contain lower case letters, numbers, and the underscore character."));
                        return false;
                }
        }
 
        if (!allow_reserved_name && IsSlotForConflictCheck(name))
        {
-               ereport(elevel,
-                               errcode(ERRCODE_RESERVED_NAME),
-                               errmsg("replication slot name \"%s\" is reserved",
-                                          name),
-                               errdetail("The name \"%s\" is reserved for the conflict detection slot.",
-                                                 CONFLICT_DETECTION_SLOT));
-
+               *err_code = ERRCODE_RESERVED_NAME;
+               *err_msg = psprintf(_("replication slot name \"%s\" is reserved"), name);
+               *err_hint = psprintf(_("The name \"%s\" is reserved for the conflict detection slot."),
+                                                        CONFLICT_DETECTION_SLOT);
                return false;
        }
 

diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index fe62162cde30468563eb8e74ab9d07cc109584e2..09c69f83d57948606f6c47e784e72743f4f63815 100644 (file)
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -321,6 +321,9 @@ extern void ReplicationSlotInitialize(void);
 extern bool ReplicationSlotValidateName(const char *name,
                                                                                bool allow_reserved_name,
                                                                                int elevel);
+extern bool ReplicationSlotValidateNameInternal(const char *name,
+                                                                                               bool allow_reserved_name,
+                                                                                               int *err_code, char **err_msg, char **err_hint);
 extern void ReplicationSlotReserveWal(void);
 extern void ReplicationSlotsComputeRequiredXmin(bool already_locked);
 extern void ReplicationSlotsComputeRequiredLSN(void);