From cb218804ef5d0cfee97df8a199eba05251462ba8 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 11 Dec 2016 09:14:53 +0200 Subject: [PATCH] Detect double monitoring of servers Adding a server to multiple monitors is forbidden. This should be detected and reported to the end user. The information provided by the config_runtime system to the client isn't as detailed as it could be. Some sort of an error message stack should be added so that client facing interfaces could properly report the reason for the failure. Currently the only way to detect the reason of the failure is to parse the log files. --- include/maxscale/monitor.h | 2 +- include/maxscale/service.h | 2 +- server/core/config_runtime.c | 19 +++++++++++-------- server/core/monitor.c | 21 ++++++++------------- server/core/service.c | 8 ++++++-- server/modules/routing/debugcli/debugcmd.c | 3 ++- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h index cbaf45917..37b507769 100644 --- a/include/maxscale/monitor.h +++ b/include/maxscale/monitor.h @@ -206,7 +206,7 @@ struct monitor extern MONITOR *monitor_alloc(char *, char *); extern void monitor_free(MONITOR *); extern MONITOR *monitor_find(const char *); -extern void monitorAddServer(MONITOR *mon, SERVER *server); +extern bool 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); diff --git a/include/maxscale/service.h b/include/maxscale/service.h index 37670a5ad..d10c8f0f3 100644 --- a/include/maxscale/service.h +++ b/include/maxscale/service.h @@ -240,7 +240,7 @@ bool serviceStartListener(SERVICE *service, const char *name); SERVICE* service_find(const char *name); // TODO: Change binlogrouter to use the functions in config_runtime.h -void serviceAddBackend(SERVICE *service, SERVER *server); +bool serviceAddBackend(SERVICE *service, SERVER *server); /** * @brief Check if a service uses a server diff --git a/server/core/config_runtime.c b/server/core/config_runtime.c index eb3444088..dd045e76a 100644 --- a/server/core/config_runtime.c +++ b/server/core/config_runtime.c @@ -29,23 +29,26 @@ bool runtime_link_server(SERVER *server, const char *target) SERVICE *service = service_find(target); MONITOR *monitor = service ? NULL : monitor_find(target); - if (service || monitor) + if (service) { - rval = true; - - if (service) + if (serviceAddBackend(service, server)) { - serviceAddBackend(service, server); service_serialize_servers(service); + rval = true; } - else if (monitor) + } + else if (monitor) + { + if (monitorAddServer(monitor, server)) { - monitorAddServer(monitor, server); monitor_serialize_servers(monitor); + rval = true; } + } + if (rval) + { const char *type = service ? "service" : "monitor"; - MXS_NOTICE("Added server '%s' to %s '%s'", server->unique_name, type, target); } diff --git a/server/core/monitor.c b/server/core/monitor.c index 8f52b0525..9f18037be 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -247,24 +247,17 @@ monitorStopAll() * @param mon The Monitor instance * @param server The Server to add to the monitoring */ -void -monitorAddServer(MONITOR *mon, SERVER *server) +bool monitorAddServer(MONITOR *mon, SERVER *server) { - bool new_server = true; - spinlock_acquire(&mon->lock); + bool rval = false; - for (MONITOR_SERVERS *db = mon->databases; db; db = db->next) + if (monitor_server_in_use(server)) { - if (db->server == server) - { - new_server = false; - } + MXS_ERROR("Server '%s' is already monitored.", server->unique_name); } - - spinlock_release(&mon->lock); - - if (new_server) + else { + rval = true; MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS)); MXS_ABORT_IF_NULL(db); @@ -307,6 +300,8 @@ monitorAddServer(MONITOR *mon, SERVER *server) monitorStart(mon, mon->parameters); } } + + return rval; } static void monitor_server_free(MONITOR_SERVERS *tofree) diff --git a/server/core/service.c b/server/core/service.c index ee43add49..74731edb5 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -768,15 +768,17 @@ static SERVER_REF* server_ref_create(SERVER *server) * @param service The service to add the server to * @param server The server to add */ -void -serviceAddBackend(SERVICE *service, SERVER *server) +bool serviceAddBackend(SERVICE *service, SERVER *server) { + bool rval = false; + if (!serviceHasBackend(service, server)) { SERVER_REF *new_ref = server_ref_create(server); if (new_ref) { + rval = true; spinlock_acquire(&service->spin); service->n_dbref++; @@ -816,6 +818,8 @@ serviceAddBackend(SERVICE *service, SERVER *server) spinlock_release(&service->spin); } } + + return rval; } /** diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index 215658151..113e7cbf2 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -852,7 +852,8 @@ static void cmd_AddServer(DCB *dcb, SERVER *server, char *v1, char *v2, char *v3 } else { - dcb_printf(dcb, "No service or monitor with the name '%s'\n", values[i]); + dcb_printf(dcb, "Could not add server '%s' to object '%s'. See error log for more details.\n", + server->unique_name, values[i]); } } }