From c5aa80b7347406bd68cdc4761997eb699ba214c1 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Fri, 19 Sep 2025 15:05:47 +0900 Subject: [PATCH v3] 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. Author: Fujii Masao Reviewed-by: Chao Li Discussion: https://postgr.es/m/CAHGQGwFud-cvthCTfusBfKHBS6Jj6kdAPTdLWKvP2qjUX6L_wA@mail.gmail.com --- src/backend/access/transam/xlogrecovery.c | 2 +- src/backend/replication/slot.c | 70 ++++++++++++++--------- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 346319338a0..c88c3d9edc3 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -4761,7 +4761,7 @@ bool check_primary_slot_name(char **newval, void **extra, GucSource source) { if (*newval && strcmp(*newval, "") != 0 && - !ReplicationSlotValidateName(*newval, false, WARNING)) + !ReplicationSlotValidateName(*newval, false, 0)) return false; return true; diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index fd0fdb96d42..5e3bc376750 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -258,7 +258,12 @@ ReplicationSlotShmemExit(int code, Datum arg) } /* - * Check whether the passed slot name is valid and report errors at elevel. + * Check whether the passed slot name is valid and report errors. + * + * When called from a GUC check hook, elevel must be 0. In that case, + * errors are reported as additional details or hints using + * GUC_check_errdetail() and GUC_check_errhint(). Otherwise, elevel + * must be nonzero, and errors are reported at that log level with ereport(). * * An error will be reported for a reserved replication slot name if * allow_reserved_name is set to false. @@ -266,30 +271,29 @@ ReplicationSlotShmemExit(int code, Datum arg) * 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 whether the slot name is valid or not if elevel < ERROR. */ bool ReplicationSlotValidateName(const char *name, bool allow_reserved_name, int elevel) { const char *cp; + int err_code = 0; + char *err_msg = NULL; + char *err_hint = NULL; if (strlen(name) == 0) { - ereport(elevel, - (errcode(ERRCODE_INVALID_NAME), - errmsg("replication slot name \"%s\" is too short", - name))); - return false; + err_code = ERRCODE_INVALID_NAME; + err_msg = psprintf(_("replication slot name \"%s\" is too short"), name); + goto error; } if (strlen(name) >= NAMEDATALEN) { - ereport(elevel, - (errcode(ERRCODE_NAME_TOO_LONG), - errmsg("replication slot name \"%s\" is too long", - name))); - return false; + err_code = ERRCODE_NAME_TOO_LONG; + err_msg = psprintf(_("replication slot name \"%s\" is too long"), name); + goto error; } for (cp = name; *cp; cp++) @@ -298,28 +302,42 @@ 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."))); - return false; + 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.")); + goto error; } } 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)); - - return false; + 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); + goto error; } return true; + +error: + if (elevel == 0) + { + GUC_check_errdetail("%s", err_msg); + if (err_hint != NULL) + GUC_check_errhint("%s", err_hint); + } + else + ereport(elevel, + (errcode(err_code), + errmsg("%s", err_msg), + (err_hint != NULL) ? errhint("%s", err_hint) : 0)); + + pfree(err_msg); + if (err_hint != NULL) + pfree(err_hint); + + return false; } /* -- 2.50.1