From a17aa28eedae9393ce3ba9a1b4fbe3f2fe026c51 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 12 Nov 2016 21:55:42 +0200 Subject: [PATCH] Persist changes to the list of monitored servers When a server is added to a monitor, an supplementary configuration file is generated to persist this information. This will allow dynamic modifications to server lists which will survive restarts and unexpected downtime. The monitor will only add new servers to its list of monitored servers. This prevents duplicate entries in the list and makes it safe to persist all used servers to the supplementary configuration file instead of only the ones that are not listed in the main configuration. --- include/maxscale/monitor.h | 16 ++ server/core/config.c | 66 ++++++++- server/core/monitor.c | 162 +++++++++++++++++---- server/modules/routing/debugcli/debugcmd.c | 2 + 4 files changed, 214 insertions(+), 32 deletions(-) diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h index 55e795922..1db55f8c3 100644 --- a/include/maxscale/monitor.h +++ b/include/maxscale/monitor.h @@ -233,6 +233,22 @@ connect_result_t mon_connect_to_db(MONITOR* mon, MONITOR_SERVERS *database); void mon_log_connect_error(MONITOR_SERVERS* database, connect_result_t rval); void mon_log_state_change(MONITOR_SERVERS *ptr); +/** + * @brief Serialize a monitor to a file + * + * This partially converts @c monitor into an INI format file. Only the servers + * of the monitor are serialized. This allows the monitor to keep monitoring + * the servers that were added at runtime even after a restart. + * + * NOTE: This does not persist the complete monitor configuration and requires + * that an existing monitor configuration is in the main configuration file. + * Changes to monitor parameters are not persisted. + * + * @param monitor Monitor to serialize + * @return False if the serialization of the monitor fails, true if it was successful + */ +bool monitor_serialize_servers(const MONITOR *monitor); + /** * Check if a monitor uses @c servers * @param server Server that is queried diff --git a/server/core/config.c b/server/core/config.c index 6e8abb7a1..26c1c4784 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -561,6 +561,47 @@ static bool is_directory(const char *dir) return rval; } +/** + * @brief Check if a directory contains .cnf files + * + * @param path Path to a directory + * @return True if the directory contained one or more .cnf files + */ +static bool contains_cnf_files(const char *path) +{ + bool rval = false; + glob_t matches; + const char suffix[] = "/*.cnf"; + char pattern[strlen(path) + sizeof(suffix)]; + + strcpy(pattern, path); + strcat(pattern, suffix); + int rc = glob(pattern, GLOB_NOSORT, NULL, &matches); + + switch (rc) + { + case 0: + rval = true; + break; + + case GLOB_NOSPACE: + MXS_OOM(); + break; + + case GLOB_ABORTED: + MXS_ERROR("Failed to read directory '%s'", path); + break; + + default: + ss_dassert(rc == GLOB_NOMATCH); + break; + } + + globfree(&matches); + + return rval; +} + /** * @brief Load the specified configuration file for MaxScale * @@ -605,7 +646,23 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT if (is_directory(persist_cnf)) { - rval = config_load_dir(persist_cnf, &dcontext, &ccontext); + DUPLICATE_CONTEXT p_dcontext; + /** + * We need to initialize a second duplicate context for the + * generated configuration files as the monitors and services will + * have duplicate sections. The duplicate sections are used to + * store changes to the list of servers the services and monitors + * use, and thus should not be treated as errors. + */ + if (duplicate_context_init(&p_dcontext)) + { + rval = config_load_dir(persist_cnf, &p_dcontext, &ccontext); + duplicate_context_finish(&p_dcontext); + } + else + { + rval = false; + } } if (rval) @@ -613,6 +670,13 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT if (!check_config_objects(ccontext.next) || !process_config(ccontext.next)) { rval = false; + if (contains_cnf_files(persist_cnf)) + { + MXS_WARNING("One or more generated configurations were found at '%s'. " + "If the error relates to any of the files located there, " + "remove the offending configurations from this directory.", + persist_cnf); + } } } } diff --git a/server/core/monitor.c b/server/core/monitor.c index 94f930a8a..39c2c14f1 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -40,6 +40,7 @@ #include #include #include +#include /* * Create declarations of the enum for monitor events and also the array of @@ -244,46 +245,62 @@ monitorStopAll() void monitorAddServer(MONITOR *mon, SERVER *server) { - MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS)); - MXS_ABORT_IF_NULL(db); - - db->server = server; - db->con = NULL; - db->next = NULL; - db->mon_err_count = 0; - db->log_version_err = true; - /** Server status is uninitialized */ - db->mon_prev_status = -1; - /* pending status is updated by get_replication_tree */ - db->pending_status = 0; - - monitor_state_t old_state = mon->state; - - if (old_state == MONITOR_STATE_RUNNING) - { - monitorStop(mon); - } - + bool new_server = true; spinlock_acquire(&mon->lock); - if (mon->databases == NULL) + for (MONITOR_SERVERS *db = mon->databases; db; db = db->next) { - mon->databases = db; - } - else - { - MONITOR_SERVERS *ptr = mon->databases; - while (ptr->next != NULL) + if (db->server == server) { - ptr = ptr->next; + new_server = false; } - ptr->next = db; } + spinlock_release(&mon->lock); - if (old_state == MONITOR_STATE_RUNNING) + if (new_server) { - monitorStart(mon, mon->parameters); + MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS)); + MXS_ABORT_IF_NULL(db); + + db->server = server; + db->con = NULL; + db->next = NULL; + db->mon_err_count = 0; + db->log_version_err = true; + /** Server status is uninitialized */ + db->mon_prev_status = -1; + /* pending status is updated by get_replication_tree */ + db->pending_status = 0; + + monitor_state_t old_state = mon->state; + + if (old_state == MONITOR_STATE_RUNNING) + { + monitorStop(mon); + } + + spinlock_acquire(&mon->lock); + + if (mon->databases == NULL) + { + mon->databases = db; + } + else + { + MONITOR_SERVERS *ptr = mon->databases; + while (ptr->next != NULL) + { + ptr = ptr->next; + } + ptr->next = db; + } + spinlock_release(&mon->lock); + + if (old_state == MONITOR_STATE_RUNNING) + { + monitorStart(mon, mon->parameters); + } } } @@ -1152,3 +1169,86 @@ bool monitor_server_in_use(const SERVER *server) return rval; } + +/** + * Creates a monitor configuration at the location pointed by @c filename + * + * @param monitor Monitor to serialize into a configuration + * @param filename Filename where configuration is written + * @return True on success, false on error + */ +static bool create_monitor_config(const MONITOR *monitor, const char *filename) +{ + int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + + if (file == -1) + { + char errbuf[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to open file '%s' when serializing monitor '%s': %d, %s", + filename, monitor->name, errno, strerror_r(errno, errbuf, sizeof(errbuf))); + return false; + } + + /** + * Only additional parameters are added to the configuration. This prevents + * duplication or addition of parameters that don't support it. + * + * TODO: Check for return values on all of the dprintf calls + */ + dprintf(file, "[%s]\n", monitor->name); + + if (monitor->databases) + { + dprintf(file, "servers="); + for (MONITOR_SERVERS *db = monitor->databases; db; db = db->next) + { + if (db != monitor->databases) + { + dprintf(file, ","); + } + dprintf(file, "%s", db->server->unique_name); + } + dprintf(file, "\n"); + } + + close(file); + + return true; +} + +bool monitor_serialize_servers(const MONITOR *monitor) +{ + bool rval = false; + char filename[PATH_MAX]; + snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(), + monitor->name); + + if (unlink(filename) == -1 && errno != ENOENT) + { + char err[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to remove temporary monitor configuration at '%s': %d, %s", + filename, errno, strerror_r(errno, err, sizeof(err))); + } + else if (create_monitor_config(monitor, filename)) + { + char final_filename[PATH_MAX]; + strcpy(final_filename, filename); + + char *dot = strrchr(final_filename, '.'); + ss_dassert(dot); + *dot = '\0'; + + if (rename(filename, final_filename) == 0) + { + rval = true; + } + else + { + char err[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to rename temporary monitor configuration at '%s': %d, %s", + filename, errno, strerror_r(errno, err, sizeof(err))); + } + } + + return rval; +} diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index 47dde9896..19e33470e 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -753,6 +753,7 @@ static void cmd_AddServer(DCB *dcb, void *a, void *b) else if (monitor) { monitorAddServer(monitor, server); + monitor_serialize_servers(monitor); } const char *target = service ? "service" : "monitor"; @@ -809,6 +810,7 @@ static void cmd_RemoveServer(DCB *dcb, void *a, void *b) else if (monitor) { monitorRemoveServer(monitor, server); + monitor_serialize_servers(monitor); } const char *target = service ? "service" : "monitor";