public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nathan Bossart <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: bt23nguyent <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: [email protected]
Subject: Re: Improve the log message output of basic_archive when basic_archive.archive_directory parameter is not set
Date: Fri, 5 Jan 2024 17:03:57 -0600
Message-ID: <20240105230357.GA2425960@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<20230915143827.GA1912920@nathanxps13>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<20230925222047.GA3173226@nathanxps13>
	<[email protected]>
	<20231013022559.GA1266364@nathanxps13>
	<[email protected]>

On Fri, Oct 13, 2023 at 11:02:39AM +0200, Daniel Gustafsson wrote:
> That's a more compelling reason IMO.  I'm not sure if I prefer the
> GUC_check_errdetail-like approach better, I would for sure not be opposed to
> reviewing a version of the patch doing it that way.
> 
> Tung Nguyen: are you interested in updating the patch along these lines
> suggested by Nathan?

I gave it a try.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com


Attachments:

  [text/x-diff] v4-0001-add-support-for-emitting-errdetail-from-archive-m.patch (2.8K, ../20240105230357.GA2425960@nathanxps13/2-v4-0001-add-support-for-emitting-errdetail-from-archive-m.patch)
  download | inline diff:
From 7183005bb6786b63a5fd96ba5101849eb6f203e5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Fri, 5 Jan 2024 17:01:10 -0600
Subject: [PATCH v4 1/1] add support for emitting errdetail from archive module
 check-configured callback

---
 contrib/basic_archive/basic_archive.c | 8 +++++++-
 src/backend/postmaster/pgarch.c       | 8 +++++++-
 src/include/archive/archive_module.h  | 8 ++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 804567e919..2c8721ebf6 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -161,7 +161,13 @@ check_archive_directory(char **newval, void **extra, GucSource source)
 static bool
 basic_archive_configured(ArchiveModuleState *state)
 {
-	return archive_directory != NULL && archive_directory[0] != '\0';
+	if (archive_directory == NULL || archive_directory[0] == '\0')
+	{
+		arch_module_check_errdetail("basic_archive.archive_directory is not set.");
+		return false;
+	}
+
+	return true;
 }
 
 /*
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 67693b0580..f663965d89 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -91,6 +91,7 @@ typedef struct PgArchData
 } PgArchData;
 
 char	   *XLogArchiveLibrary = "";
+char	   *arch_module_check_errdetail_string;
 
 
 /* ----------
@@ -408,12 +409,17 @@ pgarch_ArchiverCopyLoop(void)
 			 */
 			HandlePgArchInterrupts();
 
+			/* Reset variables that might be set by the callback */
+			arch_module_check_errdetail_string = NULL;
+
 			/* can't do anything if not configured ... */
 			if (ArchiveCallbacks->check_configured_cb != NULL &&
 				!ArchiveCallbacks->check_configured_cb(archive_module_state))
 			{
 				ereport(WARNING,
-						(errmsg("archive_mode enabled, yet archiving is not configured")));
+						(errmsg("archive_mode enabled, yet archiving is not configured"),
+						 arch_module_check_errdetail_string ?
+						 errdetail_internal("%s", arch_module_check_errdetail_string) : 0));
 				return;
 			}
 
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
index fd59b9faf4..763af76e54 100644
--- a/src/include/archive/archive_module.h
+++ b/src/include/archive/archive_module.h
@@ -56,4 +56,12 @@ typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
 
 extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
 
+/* Support for messages reported from archive module callbacks. */
+
+extern PGDLLIMPORT char *arch_module_check_errdetail_string;
+
+#define arch_module_check_errdetail \
+	pre_format_elog_string(errno, TEXTDOMAIN), \
+	arch_module_check_errdetail_string = format_elog_string
+
 #endif							/* _ARCHIVE_MODULE_H */
-- 
2.25.1

view thread (11+ 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], [email protected], [email protected], [email protected]
  Subject: Re: Improve the log message output of basic_archive when basic_archive.archive_directory parameter is not set
  In-Reply-To: <20240105230357.GA2425960@nathanxps13>

* 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