From ae84041a4e1cc689896fbcf7dd1edbe73143ec84 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Fri, 13 Nov 2015 14:29:11 +0900 Subject: [PATCH] Add log_hint() function for logging hints There are a few places where additional hints are written as log output, usually LOG_NOTICE. Create an explicit function to provide hints in a standardized manner; by storing the log level of the previous logger call, we can ensure the hint is only displayed when the log message itself would be. Part of an ongoing effort to better control repmgr's logging output. --- log.c | 18 +++++++++++++++++- log.h | 3 ++- repmgr.c | 18 ++++++++++-------- repmgrd.c | 2 +- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/log.c b/log.c index 6ad5427e..bca86ae5 100644 --- a/log.c +++ b/log.c @@ -44,7 +44,7 @@ static int detect_log_facility(const char *facility); int log_type = REPMGR_STDERR; int log_level = LOG_NOTICE; - +int last_log_level = LOG_NOTICE; void stderr_log_with_level(const char *level_name, int level, const char *fmt, ...) @@ -54,6 +54,12 @@ stderr_log_with_level(const char *level_name, int level, const char *fmt, ...) char buff[100]; va_list ap; + /* + * Store the requested level so that if there's a subsequent + * log_hint(), we can suppress that if appropriate. + */ + last_log_level = level; + if (log_level >= level) { time(&t); @@ -69,6 +75,16 @@ stderr_log_with_level(const char *level_name, int level, const char *fmt, ...) } } +void +log_hint(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + stderr_log_with_level("HINT", last_log_level, fmt, ap); + va_end(ap); +} + bool logger_init(t_configuration_options * opts, const char *ident, const char *level, const char *facility) diff --git a/log.h b/log.h index f6b80c70..4dcee307 100644 --- a/log.h +++ b/log.h @@ -115,10 +115,11 @@ __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4))); /* Logger initialisation and shutdown */ bool logger_shutdown(void); -bool logger_init(t_configuration_options * opts, const char *ident, +bool logger_init(t_configuration_options * opts, const char *ident, const char *level, const char *facility); void logger_min_verbose(int minimum); +void log_hint(const char *fmt, ...); extern int log_type; extern int log_level; diff --git a/repmgr.c b/repmgr.c index 7c2cc669..81952dc2 100644 --- a/repmgr.c +++ b/repmgr.c @@ -495,6 +495,7 @@ main(int argc, char **argv) * logging level might be specified at, but it often requires detailed * logging to troubleshoot problems. */ + logger_init(&options, progname(), options.loglevel, options.logfacility); if (runtime_options.verbose) logger_min_verbose(LOG_INFO); @@ -616,7 +617,7 @@ do_cluster_show(void) { log_err(_("Unable to retrieve node information from the database\n%s\n"), PQerrorMessage(conn)); - log_notice(_("HINT: Please check that all nodes have been registered\n")); + log_hint(_("Please check that all nodes have been registered\n")); PQclear(res); PQfinish(conn); @@ -965,9 +966,10 @@ do_standby_register(void) { if (!runtime_options.force) { - log_notice(_("HINT: use option -F/--force to overwrite an existing node record\n")); + log_hint(_("use option -F/--force to overwrite an existing node record\n")); } + // XXX log registration failure? PQfinish(master_conn); PQfinish(conn); exit(ERR_BAD_CONFIG); @@ -1338,7 +1340,7 @@ do_standby_clone(void) { log_err(_("unable to use directory %s ...\n"), local_data_directory); - log_notice(_("HINT: Use -F/--force option to force this directory to be overwritten\n")); + log_hint(_("use -F/--force option to force this directory to be overwritten\n")); r = ERR_BAD_CONFIG; retval = ERR_BAD_CONFIG; goto stop_backup; @@ -1720,10 +1722,10 @@ stop_backup: * - provide a custom pg_ctl command */ - log_notice(_("HINT: you can now start your PostgreSQL server\n")); + log_notice(_("you can now start your PostgreSQL server\n")); if (target_directory_provided) { - log_notice(_("for example : pg_ctl -D %s start\n"), + log_hint(_("for example : pg_ctl -D %s start\n"), local_data_directory); } else @@ -3475,7 +3477,7 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error) if (i == 0) { log_err(_("parameter 'max_replication_slots' must be set to at least 1 to enable replication slots\n")); - log_notice(_("HINT: 'max_replication_slots' should be set to at least the number of expected standbys\n")); + log_hint(_("'max_replication_slots' should be set to at least the number of expected standbys\n")); if (exit_on_error == true) { PQfinish(conn); @@ -3504,7 +3506,7 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error) runtime_options.wal_keep_segments); if (server_version_num >= 90400) { - log_notice(_("HINT: in PostgreSQL 9.4 and later, replication slots can be used, which " + log_hint(_("in PostgreSQL 9.4 and later, replication slots can be used, which " "do not require 'wal_keep_segments' to be set to a high value " "(set parameter 'use_replication_slots' in the configuration file to enable)\n" )); @@ -3587,7 +3589,7 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error) if (i == 0) { log_err(_("parameter 'max_wal_senders' must be set to be at least 1\n")); - log_notice(_("HINT: 'max_wal_senders' should be set to at least the number of expected standbys\n")); + log_hint(_("'max_wal_senders' should be set to at least the number of expected standbys\n")); } if (exit_on_error == true) diff --git a/repmgrd.c b/repmgrd.c index a0706150..e274ebe5 100644 --- a/repmgrd.c +++ b/repmgrd.c @@ -281,7 +281,7 @@ main(int argc, char **argv) if (node_info.node_id == NODE_NOT_FOUND) { log_err(_("No metadata record found for this node - terminating\n")); - log_notice(_("HINT: was this node registered with 'repmgr (master|standby) register'?\n")); + log_hint(_("Check that 'repmgr (master|standby) register' was executed for this node\n")); terminate(ERR_BAD_CONFIG); }