diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 544e5d8a6..e0da08d49 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -745,23 +745,16 @@ bool do_alter_monitor(Monitor* monitor, const char* key, const char* value) return false; } - // Backup monitor parameters in case configure fails. - MXS_CONFIG_PARAMETER originals = monitor->parameters; MXS_CONFIG_PARAMETER modified = monitor->parameters; - modified.set(key, value); - bool success = monitor->configure(&modified); + + bool success = MonitorManager::reconfigure_monitor(monitor, modified); + if (success) { MXS_NOTICE("Updated monitor '%s': %s=%s", monitor->m_name, key, value); } - else - { - // Configure failed, restore original configs. This should not fail. - // TODO: add a flag to monitor which prevents startup if config is wrong. - MXB_AT_DEBUG(bool check = ) monitor->configure(&originals); - mxb_assert(check); - } + return success; } @@ -2535,17 +2528,11 @@ bool runtime_alter_monitor_from_json(Monitor* monitor, json_t* new_json) MonitorManager::stop_monitor(monitor); auto old_params = monitor->parameters; - if (monitor->configure(¶ms)) + if (MonitorManager::reconfigure_monitor(monitor, params)) { MonitorManager::monitor_serialize(monitor); success = true; } - else - { - // Something went wrong, restore the old parameters - MXB_AT_DEBUG(bool check = ) monitor->configure(&old_params); - mxb_assert(check); - } if (restart) { diff --git a/server/core/internal/monitor.hh b/server/core/internal/monitor.hh index d3783b4bc..1c5d9c543 100644 --- a/server/core/internal/monitor.hh +++ b/server/core/internal/monitor.hh @@ -149,6 +149,18 @@ public: */ static bool monitor_serialize(const mxs::Monitor* monitor); + /** + * Attempt to reconfigure a monitor + * + * If the configuration fails, the old parameters are restored. + * + * @param monitor Monitor to reconfigure + * @param parameters New parameters to apply + * + * @return True if reconfiguration was successful + */ + static bool reconfigure_monitor(mxs::Monitor* monitor, const MXS_CONFIG_PARAMETER& parameters); + /** * @brief Convert monitor to JSON * diff --git a/server/core/monitor.cc b/server/core/monitor.cc index ca4dac34e..6d28cbba7 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -792,6 +792,24 @@ bool MonitorManager::monitor_serialize(const Monitor* monitor) return rval; } +// static +bool MonitorManager::reconfigure_monitor(mxs::Monitor* monitor, const MXS_CONFIG_PARAMETER& parameters) +{ + // Backup monitor parameters in case configure fails. + auto orig = monitor->parameters; + monitor->parameters.clear(); + + bool success = monitor->configure(¶meters); + + if (!success) + { + MXB_AT_DEBUG(bool check = ) monitor->configure(&orig); + mxb_assert(check); + } + + return success; +} + json_t* MonitorManager::monitor_to_json(const Monitor* monitor, const char* host) { string self = MXS_JSON_API_MONITORS;