From 4c4bd24a406539b6f58f3beb4bc810b2103f2e92 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 29 Nov 2016 16:25:45 +0200 Subject: [PATCH] Allow module specific monitor parameters to be altered Module specific parameters can now be altered at runtime. This allows both the removal and addition of arbitrary monitor parameters. --- include/maxscale/config.h | 2 +- include/maxscale/monitor.h | 2 ++ server/core/config.c | 4 +-- server/core/config_runtime.c | 31 +++++++++++++++++++--- server/core/monitor.c | 29 +++++++++++++++++++- server/core/service.c | 2 +- server/modules/routing/debugcli/debugcmd.c | 12 ++++++--- 7 files changed, 71 insertions(+), 11 deletions(-) diff --git a/include/maxscale/config.h b/include/maxscale/config.h index 6f02ec613..afb1a0029 100644 --- a/include/maxscale/config.h +++ b/include/maxscale/config.h @@ -238,7 +238,7 @@ bool config_set_qualified_param(CONFIG_PARAMETER* param, config_param_type_t type); int config_threadcount(); int config_truth_value(char *); -void free_config_parameter(CONFIG_PARAMETER* p1); +void config_parameter_free(CONFIG_PARAMETER* p1); bool is_internal_service(const char *router); MXS_END_DECLS diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h index 70dcd7b63..cbaf45917 100644 --- a/include/maxscale/monitor.h +++ b/include/maxscale/monitor.h @@ -199,6 +199,7 @@ struct monitor char *module_name; /**< Name of the monitor module */ void *handle; /**< Handle returned from startMonitor */ size_t interval; /**< The monitor interval */ + bool created_online; /**< Whether this monitor was created at runtime */ struct monitor *next; /**< Next monitor in the linked list */ }; @@ -209,6 +210,7 @@ extern void monitorAddServer(MONITOR *mon, SERVER *server); extern void monitorRemoveServer(MONITOR *mon, SERVER *server); extern void monitorAddUser(MONITOR *, char *, char *); extern void monitorAddParameters(MONITOR *monitor, CONFIG_PARAMETER *params); +extern bool monitorRemoveParameter(MONITOR *monitor, const char *key); extern void monitorStop(MONITOR *); extern void monitorStart(MONITOR *, void*); extern void monitorStopAll(); diff --git a/server/core/config.c b/server/core/config.c index 51e1a82ec..70ae9c77e 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -1070,7 +1070,7 @@ return_p2: * Free a configuration parameter * @param p1 Parameter to free */ -void free_config_parameter(CONFIG_PARAMETER* p1) +void config_parameter_free(CONFIG_PARAMETER* p1) { while (p1) { @@ -1089,7 +1089,7 @@ void config_context_free(CONFIG_CONTEXT *context) while (context) { obj = context->next; - free_config_parameter(context->parameters); + config_parameter_free(context->parameters); MXS_FREE(context->object); MXS_FREE(context); context = obj; diff --git a/server/core/config_runtime.c b/server/core/config_runtime.c index a5f5dcb8f..c5900990a 100644 --- a/server/core/config_runtime.c +++ b/server/core/config_runtime.c @@ -361,8 +361,27 @@ bool runtime_alter_monitor(MONITOR *monitor, char *key, char *value) monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival); } } + else + { + /** We're modifying module specific parameters and we need to stop the monitor */ + monitorStop(monitor); - if (valid) + if (monitorRemoveParameter(monitor, key) || value[0]) + { + /** Either we're removing an existing parameter or adding a new one */ + valid = true; + + if (value[0]) + { + CONFIG_PARAMETER p = {.name = key, .value = value}; + monitorAddParameters(monitor, &p); + } + } + + monitorStart(monitor, monitor->parameters); + } + + if (valid && monitor->created_online) { monitor_serialize(monitor); } @@ -497,9 +516,15 @@ bool runtime_create_monitor(const char *name, const char *module) bool rval = false; MONITOR *monitor = monitor_alloc((char*)name, (char*)module); - if (monitor && monitor_serialize(monitor)) + if (monitor) { - rval = true; + /** Mark that this monitor was created after MaxScale was started */ + monitor->created_online = true; + + if (monitor_serialize(monitor)) + { + rval = true; + } } spinlock_release(&crt_lock); diff --git a/server/core/monitor.c b/server/core/monitor.c index 9542b912b..cb02f5deb 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -104,6 +104,7 @@ monitor_alloc(char *name, char *module) mon->connect_timeout = DEFAULT_CONNECT_TIMEOUT; mon->interval = MONITOR_INTERVAL; mon->parameters = NULL; + mon->created_online = false; spinlock_init(&mon->lock); spinlock_acquire(&monLock); mon->next = allMonitors; @@ -144,7 +145,7 @@ monitor_free(MONITOR *mon) } } spinlock_release(&monLock); - free_config_parameter(mon->parameters); + config_parameter_free(mon->parameters); monitor_server_free_all(mon->databases); MXS_FREE(mon->name); MXS_FREE(mon->module_name); @@ -767,6 +768,32 @@ void monitorAddParameters(MONITOR *monitor, CONFIG_PARAMETER *params) } } +bool monitorRemoveParameter(MONITOR *monitor, const char *key) +{ + CONFIG_PARAMETER *prev = NULL; + + for (CONFIG_PARAMETER *p = monitor->parameters; p; p = p->next) + { + if (strcmp(p->name, key) == 0) + { + if (p == monitor->parameters) + { + monitor->parameters = monitor->parameters->next; + p->next = NULL; + } + else + { + prev->next = p->next; + p->next = NULL; + } + config_parameter_free(p); + return true; + } + prev = p; + } + return false; +} + /** * Set a pending status bit in the monitor server * diff --git a/server/core/service.c b/server/core/service.c index 0c7165e64..310fd712f 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -665,7 +665,7 @@ void service_free(SERVICE *service) MXS_FREE(service->credentials.name); MXS_FREE(service->credentials.authdata); - free_config_parameter(service->svc_config_param); + config_parameter_free(service->svc_config_param); serviceClearRouterOptions(service); MXS_FREE(service); diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index cad322c67..818756553 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -1290,6 +1290,13 @@ static void alterMonitor(DCB *dcb, MONITOR *monitor, char *v1, char *v2, char *v { dcb_printf(dcb, "Error: Bad key-value parameter: %s=%s\n", key, value); } + else if (!monitor->created_online) + { + dcb_printf(dcb, "Warning: Altered monitor '%s' which is in the " + "main\nconfiguration file. These changes will not be " + "persisted and need\nto be manually added or set again" + "after a restart.\n", monitor->name); + } } else { @@ -1316,9 +1323,8 @@ struct subcommand alteroptions[] = "monitor", 2, 12, alterMonitor, "Alter monitor parameters", "Usage: alter monitor NAME KEY=VALUE ...\n" - "This will alter an existing parameter of a monitor. The accepted values\n" - "for KEY are: 'user', 'password', 'monitor_interval',\n" - "'backend_connect_timeout', 'backend_write_timeout', 'backend_read_timeout'\n" + "This will alter an existing parameter of a monitor. To remove parameters,\n" + "pass an empty value for a key e.g. 'maxadmin alter monitor my-monitor my-key='\n" "A maximum of 11 parameters can be changed at one time", {ARG_TYPE_MONITOR, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,