pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: bt23nguyent <bt23nguyent@oss.nttdata.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@lists.postgresql.org
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: <E863EDCA-EFEA-4DFA-9E93-024D464DE260@yesql.se>
References: <202309151049.dmfsxc5b3wyf@alvherre.pgsql>
	<7035FAFE-7E91-4B31-AA72-75EE82DB3F7B@yesql.se>
	<20230915143827.GA1912920@nathanxps13>
	<85b9e69997bbd5d8302a3a99a2de246d@oss.nttdata.com>
	<D3038255-EAB5-4164-A9E3-7967B6738993@yesql.se>
	<18cbd8ca6f5390cda2920644f891a419@oss.nttdata.com>
	<20230925222047.GA3173226@nathanxps13>
	<482056B1-D930-417C-9473-CB1A045E42AE@yesql.se>
	<20231013022559.GA1266364@nathanxps13>
	<E863EDCA-EFEA-4DFA-9E93-024D464DE260@yesql.se>

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:

v4-0001-add-support-for-emitting-errdetail-from-archive-m.patch (text/x-diff, 2.8K) download From 7183005bb6786b63a5fd96ba5101849eb6f203e5 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> 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 (22+ messages)  latest in thread

Message-ID: <20240105230357.GA2425960@nathanxps13>
Permalink:  ../20240105230357.GA2425960@nathanxps13/
Also on:    postgresql.org/message-id/20240105230357.GA2425960@nathanxps13

 ·  · 

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: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, daniel@yesql.se, bt23nguyent@oss.nttdata.com, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org
  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 DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox