[bt23nguyent <bt23nguyent@oss.nttdata.com>]

Hi,

When archive_library is set to 'basic_archive' but 
basic_archive.archive_directory is not set, WAL archiving doesn't work 
and only the following warning message is logged.

     $ emacs $PGDATA/postgresql.conf
     archive_mode = on
     archive_library = 'basic_archive'

     $ bin/pg_ctl -D $PGDATA restart
     ....
     WARNING:  archive_mode enabled, yet archiving is not configured

The issue here is that this warning message doesn't suggest any hint 
regarding the cause of WAL archiving failure. In other words, I think 
that the log message in this case should report that WAL archiving 
failed because basic_archive.archive_directory is not set. Thus, I think 
it's worth implementing new patch that improves that warning message, 
and here is the patch for that.

Best regards,
Tung Nguyen

Attachments:

v1-0001-Improve-log-message-output-of-basic-archive.patch (text/x-diff, 2.9K) download
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 4d78c31859..dedc53c367 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -48,7 +48,7 @@ typedef struct BasicArchiveData
 static char *archive_directory = NULL;
 
 static void basic_archive_startup(ArchiveModuleState *state);
-static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state, const char **errmsg);
 static bool basic_archive_file(ArchiveModuleState *state, const char *file, const char *path);
 static void basic_archive_file_internal(const char *file, const char *path);
 static bool check_archive_directory(char **newval, void **extra, GucSource source);
@@ -159,9 +159,15 @@ check_archive_directory(char **newval, void **extra, GucSource source)
  * Checks that archive_directory is not blank.
  */
 static bool
-basic_archive_configured(ArchiveModuleState *state)
+basic_archive_configured(ArchiveModuleState *state, const char **errmsg)
 {
-	return archive_directory != NULL && archive_directory[0] != '\0';
+	if (archive_directory == NULL || archive_directory[0] == '\0')
+    {
+        *errmsg = "WAL archiving failed because basic_archive.archive_directory is not set";
+        return false;
+    }
+    else
+        return true;
 }
 
 /*
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 46af349564..100e33f48d 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -91,6 +91,7 @@ typedef struct PgArchData
 } PgArchData;
 
 char	   *XLogArchiveLibrary = "";
+char       *msg;
 
 
 /* ----------
@@ -410,10 +411,11 @@ pgarch_ArchiverCopyLoop(void)
 
 			/* can't do anything if not configured ... */
 			if (ArchiveCallbacks->check_configured_cb != NULL &&
-				!ArchiveCallbacks->check_configured_cb(archive_module_state))
+				!ArchiveCallbacks->check_configured_cb(archive_module_state, &msg))
 			{
 				ereport(WARNING,
-						(errmsg("archive_mode enabled, yet archiving is not configured")));
+						(errmsg("archive_mode enabled, yet archiving is not configured")),
+						(msg != NULL) ? errhint("%s", msg) : 0);
 				return;
 			}
 
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
index 679ce5a6db..7f7fbeae8a 100644
--- a/src/include/archive/archive_module.h
+++ b/src/include/archive/archive_module.h
@@ -36,7 +36,7 @@ typedef struct ArchiveModuleState
  * archive modules documentation.
  */
 typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
-typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state, const char **errmsg);
 typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
 typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);