From 07e58444f65d9c6d781861a7fb836340345eeae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 17 Nov 2017 17:09:49 +0200 Subject: [PATCH] Improve error message for zero monitor timeout values The error message was not 100% accurate about the value. In addition to that, neither the value itself nor the monitor or parameter names were printed in the error message. --- server/core/config.cc | 12 ++++++++---- server/core/config_runtime.cc | 8 ++++---- server/core/maxscale/monitor.h | 2 +- server/core/monitor.cc | 6 +++--- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/server/core/config.cc b/server/core/config.cc index b3a326090..2bb3f5990 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -3239,7 +3239,8 @@ int create_new_monitor(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj, HASHTABLE* char *connect_timeout = config_get_value(obj->parameters, CN_BACKEND_CONNECT_TIMEOUT); if (connect_timeout) { - if (!monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_TIMEOUT, atoi(connect_timeout))) + if (!monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_TIMEOUT, + atoi(connect_timeout), CN_BACKEND_CONNECT_TIMEOUT)) { MXS_ERROR("Failed to set '%s'", CN_BACKEND_CONNECT_TIMEOUT); error_count++; @@ -3249,7 +3250,8 @@ int create_new_monitor(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj, HASHTABLE* char *read_timeout = config_get_value(obj->parameters, CN_BACKEND_READ_TIMEOUT); if (read_timeout) { - if (!monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, atoi(read_timeout))) + if (!monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, + atoi(read_timeout), CN_BACKEND_READ_TIMEOUT)) { MXS_ERROR("Failed to set '%s'", CN_BACKEND_READ_TIMEOUT); error_count++; @@ -3259,7 +3261,8 @@ int create_new_monitor(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj, HASHTABLE* char *write_timeout = config_get_value(obj->parameters, CN_BACKEND_WRITE_TIMEOUT); if (write_timeout) { - if (!monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, atoi(write_timeout))) + if (!monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, + atoi(write_timeout), CN_BACKEND_WRITE_TIMEOUT)) { MXS_ERROR("Failed to set '%s'", CN_BACKEND_WRITE_TIMEOUT); error_count++; @@ -3269,7 +3272,8 @@ int create_new_monitor(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj, HASHTABLE* char *connect_attempts = config_get_value(obj->parameters, CN_BACKEND_CONNECT_ATTEMPTS); if (connect_attempts) { - if (!monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_ATTEMPTS, atoi(connect_attempts))) + if (!monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_ATTEMPTS, + atoi(connect_attempts), CN_BACKEND_CONNECT_ATTEMPTS)) { MXS_ERROR("Failed to set '%s'", CN_BACKEND_CONNECT_ATTEMPTS); error_count++; diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 2882e90ee..d6507d5be 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -474,7 +474,7 @@ bool runtime_alter_monitor(MXS_MONITOR *monitor, const char *key, const char *va if (ival) { valid = true; - monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_TIMEOUT, ival); + monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_TIMEOUT, ival, CN_BACKEND_CONNECT_TIMEOUT); } } else if (strcmp(key, CN_BACKEND_WRITE_TIMEOUT) == 0) @@ -483,7 +483,7 @@ bool runtime_alter_monitor(MXS_MONITOR *monitor, const char *key, const char *va if (ival) { valid = true; - monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, ival); + monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, ival, CN_BACKEND_WRITE_TIMEOUT); } } else if (strcmp(key, CN_BACKEND_READ_TIMEOUT) == 0) @@ -492,7 +492,7 @@ bool runtime_alter_monitor(MXS_MONITOR *monitor, const char *key, const char *va if (ival) { valid = true; - monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival); + monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival, CN_BACKEND_READ_TIMEOUT); } } else if (strcmp(key, CN_BACKEND_CONNECT_ATTEMPTS) == 0) @@ -501,7 +501,7 @@ bool runtime_alter_monitor(MXS_MONITOR *monitor, const char *key, const char *va if (ival) { valid = true; - monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_ATTEMPTS, ival); + monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_ATTEMPTS, ival, CN_BACKEND_CONNECT_ATTEMPTS); } } else if (strcmp(key, CN_JOURNAL_MAX_AGE) == 0) diff --git a/server/core/maxscale/monitor.h b/server/core/maxscale/monitor.h index 48a10ddf2..319219251 100644 --- a/server/core/maxscale/monitor.h +++ b/server/core/maxscale/monitor.h @@ -72,7 +72,7 @@ void monitorAddParameters(MXS_MONITOR *monitor, MXS_CONFIG_PARAMETER *params); bool monitorRemoveParameter(MXS_MONITOR *monitor, const char *key); void monitorSetInterval (MXS_MONITOR *, unsigned long); -bool monitorSetNetworkTimeout(MXS_MONITOR *, int, int); +bool monitorSetNetworkTimeout(MXS_MONITOR *, int, int, const char*); void monitorSetJournalMaxAge(MXS_MONITOR *mon, time_t value); void monitorSetScriptTimeout(MXS_MONITOR *mon, uint32_t value); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 2a2168155..13d6f90ba 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -668,8 +668,7 @@ void monitorSetScriptTimeout(MXS_MONITOR *mon, uint32_t value) * @param type The timeout handling type * @param value The timeout to set */ -bool -monitorSetNetworkTimeout(MXS_MONITOR *mon, int type, int value) +bool monitorSetNetworkTimeout(MXS_MONITOR *mon, int type, int value, const char* key) { bool rval = true; @@ -695,13 +694,14 @@ monitorSetNetworkTimeout(MXS_MONITOR *mon, int type, int value) default: MXS_ERROR("Monitor setNetworkTimeout received an unsupported action type %i", type); + ss_dassert(!true); rval = false; break; } } else { - MXS_ERROR("Negative value for monitor timeout."); + MXS_ERROR("Value '%s' for monitor '%s' is not a positive integer: %d", key, mon->name, value); rval = false; } return rval;