diff --git a/include/maxscale/config.hh b/include/maxscale/config.hh index c217ba3aa..3227c3eca 100644 --- a/include/maxscale/config.hh +++ b/include/maxscale/config.hh @@ -598,7 +598,7 @@ bool config_set_writeq_low_water(uint32_t size); * @return True, if @ config_value was valid, false otherwise. * */ -bool config_parse_disk_space_threshold(MxsDiskSpaceThreshold* disk_space_threshold, +bool config_parse_disk_space_threshold(SERVER::DiskSpaceLimits* disk_space_threshold, const char* config_value); diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index ec1580c34..7826ac1f2 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -200,6 +200,14 @@ public: */ virtual json_t* diagnostics_json() const = 0; + /** + * Set disk space threshold setting. + * + * @param dst_setting The disk space threshold as specified in the config file. + * @return True, if the provided string is valid and the threshold could be set. + */ + bool set_disk_space_threshold(const std::string& dst_setting); + const char* const name; /**< Monitor instance name. TODO: change to string */ const std::string module_name; /**< Name of the monitor module */ bool active = true; /**< True if monitor exists and has not been "destroyed". */ @@ -235,9 +243,22 @@ public: const char* script; /**< Launchable script. */ uint64_t events; /**< Enabled monitor events. */ - MxsDiskSpaceThreshold* disk_space_threshold = NULL; /**< Disk space thresholds */ - int64_t disk_space_check_interval = -1; /**< How often should a disk space check be made - * at most. */ +protected: + /** + * Contains monitor base class settings. Since monitors are stopped before a setting change, + * the items cannot be modified while a monitor is running. No locking required. + */ + class Settings + { + public: + SERVER::DiskSpaceLimits disk_space_limits; /**< Disk space thresholds */ + /** + * How often should a disk space check be made at most, in milliseconds. Negative values imply + * disabling. */ + int64_t disk_space_check_interval = -1; + }; + + Settings m_settings; private: friend class MonitorManager; @@ -414,16 +435,6 @@ int mon_config_get_servers(const MXS_CONFIG_PARAMETER* params, const Monitor* mon, MXS_MONITORED_SERVER*** monitored_array_out); -/** - * @brief Set the disk space threshold of a monitor - * - * @param server The monitor. - * @param disk_space_threshold The disk space threshold as specified in the config file. - * - * @return True, if the provided string is valid and the threshold could be set. - */ -bool monitor_set_disk_space_threshold(Monitor* monitor, const char* disk_space_threshold); - // Function for waiting one monitor interval void monitor_debug_wait(); diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index 42a21c401..a54230732 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -20,9 +20,6 @@ #include #include -// A mapping from a path to a percentage, e.g.: "/disk" -> 80. -typedef std::unordered_map MxsDiskSpaceThreshold; - /** * Server configuration parameters names */ @@ -135,6 +132,9 @@ public: static const int MAINTENANCE_FLAG_NOCHECK = 0; static const int MAINTENANCE_FLAG_CHECK = -1; + // A mapping from a path to a percentage, e.g.: "/disk" -> 80. + typedef std::unordered_map DiskSpaceLimits; + enum class Type { MARIADB, @@ -219,7 +219,7 @@ public: * * @return A copy of settings */ - virtual MxsDiskSpaceThreshold get_disk_space_limits() const = 0; + virtual DiskSpaceLimits get_disk_space_limits() const = 0; /** * Is persistent connection pool enabled. diff --git a/server/core/config.cc b/server/core/config.cc index 5e976d24c..e969026d9 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -4803,7 +4803,7 @@ bool get_suffixed_size(const char* value, uint64_t* dest) return rval; } -bool config_parse_disk_space_threshold(MxsDiskSpaceThreshold* pDisk_space_threshold, +bool config_parse_disk_space_threshold(SERVER::DiskSpaceLimits* pDisk_space_threshold, const char* zDisk_space_threshold) { mxb_assert(pDisk_space_threshold); @@ -4813,7 +4813,7 @@ bool config_parse_disk_space_threshold(MxsDiskSpaceThreshold* pDisk_space_thresh using namespace std; - MxsDiskSpaceThreshold disk_space_threshold; + SERVER::DiskSpaceLimits disk_space_threshold; string s(zDisk_space_threshold); // Somewhat simplified, this is what we expect: [^:]+:[:digit:]+(,[^:]+:[:digit:]+)* diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 67507fbba..3b2c1cf0b 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -648,7 +648,7 @@ bool do_alter_monitor(Monitor* monitor, const char* key, const char* value) } else if (strcmp(key, CN_DISK_SPACE_THRESHOLD) == 0) { - success = monitor_set_disk_space_threshold(monitor, value); + success = monitor->set_disk_space_threshold(value); } else { diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index a5b9b361c..1e0ea2cdd 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -68,13 +68,13 @@ public: return !m_settings.disk_space_limits.empty(); } - MxsDiskSpaceThreshold get_disk_space_limits() const override + DiskSpaceLimits get_disk_space_limits() const override { std::lock_guard guard(m_settings.lock); return m_settings.disk_space_limits; } - void set_disk_space_limits(const MxsDiskSpaceThreshold& new_limits) + void set_disk_space_limits(const DiskSpaceLimits& new_limits) { std::lock_guard guard(m_settings.lock); m_settings.disk_space_limits = new_limits; @@ -333,7 +333,7 @@ private: /** Disk space thresholds. Can be queried from modules at any time so access must be protected * by mutex. */ - MxsDiskSpaceThreshold disk_space_limits; + DiskSpaceLimits disk_space_limits; /** Additional custom parameters which may affect routing decisions or the monitor module. * Can be queried from modules at any time so access must be protected by mutex. */ diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 10f9db17f..5064b05f2 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -197,7 +197,7 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) script_timeout = config_get_integer(params, CN_SCRIPT_TIMEOUT); script = config_get_string(params, CN_SCRIPT); events = config_get_enum(params, CN_EVENTS, mxs_monitor_event_enum_values); - disk_space_check_interval = config_get_integer(params, CN_DISK_SPACE_CHECK_INTERVAL); + m_settings.disk_space_check_interval = config_get_integer(params, CN_DISK_SPACE_CHECK_INTERVAL); monitor_add_user(this, config_get_string(params, CN_USER), config_get_string(params, CN_PASSWORD)); for (auto& s : mxs::strtok(config_get_string(params, CN_SERVERS), ",")) @@ -210,7 +210,7 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) * to be correct. The following is a complicated type and needs to be checked separately. */ bool error = false; const char* threshold_string = config_get_string(params, CN_DISK_SPACE_THRESHOLD); - if (!monitor_set_disk_space_threshold(this, threshold_string)) + if (!set_disk_space_threshold(threshold_string)) { MXS_ERROR("Invalid value for '%s' for monitor %s: %s", CN_DISK_SPACE_THRESHOLD, name, threshold_string); @@ -229,7 +229,6 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) Monitor::~Monitor() { - delete disk_space_threshold; config_parameter_free(parameters); monitor_server_free_all(monitored_servers); MXS_FREE((const_cast(name))); @@ -2362,26 +2361,14 @@ int mon_config_get_servers(const MXS_CONFIG_PARAMETER* params, return found; } -bool monitor_set_disk_space_threshold(Monitor* monitor, const char* disk_space_threshold) +bool Monitor::set_disk_space_threshold(const string& dst_setting) { - mxb_assert(monitor->state == MONITOR_STATE_STOPPED); - MxsDiskSpaceThreshold dst; - bool rv = config_parse_disk_space_threshold(&dst, disk_space_threshold); + mxb_assert(state == MONITOR_STATE_STOPPED); + SERVER::DiskSpaceLimits new_dst; + bool rv = config_parse_disk_space_threshold(&new_dst, dst_setting.c_str()); if (rv) { - if (!monitor->disk_space_threshold) - { - monitor->disk_space_threshold = new(std::nothrow) MxsDiskSpaceThreshold; - } - - if (monitor->disk_space_threshold) - { - monitor->disk_space_threshold->swap(dst); - } - else - { - rv = false; - } + m_settings.disk_space_limits = new_dst; } return rv; } @@ -2533,12 +2520,12 @@ bool MonitorWorker::should_update_disk_space_status(const MXS_MONITORED_SERVER* { bool should_check = false; - if ((m_monitor->disk_space_check_interval > 0) + if ((m_settings.disk_space_check_interval > 0) && (pMs->disk_space_checked != -1) // -1 means disabled - && (m_monitor->disk_space_threshold || pMs->server->have_disk_space_limits())) + && (!m_settings.disk_space_limits.empty() || pMs->server->have_disk_space_limits())) { int64_t now = get_time_ms(); - if (now - pMs->disk_space_checked > m_monitor->disk_space_check_interval) + if (now - pMs->disk_space_checked > m_settings.disk_space_check_interval) { should_check = true; } @@ -2584,10 +2571,10 @@ void MonitorWorker::update_disk_space_status(MXS_MONITORED_SERVER* pMs) if (rv == 0) { // Server-specific setting takes precedence. - MxsDiskSpaceThreshold dst = pMs->server->get_disk_space_limits(); + auto dst = pMs->server->get_disk_space_limits(); if (dst.empty()) { - dst = *m_monitor->disk_space_threshold; + dst = m_settings.disk_space_limits; } bool disk_space_exhausted = false; diff --git a/server/core/server.cc b/server/core/server.cc index 219c040bc..cb70ab153 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -1176,7 +1176,7 @@ json_t* Server::server_list_to_json(const char* host) bool Server::set_disk_space_threshold(const char* disk_space_threshold) { - MxsDiskSpaceThreshold dst; + DiskSpaceLimits dst; bool rv = config_parse_disk_space_threshold(&dst, disk_space_threshold); if (rv) { diff --git a/server/core/test/test_config.cc b/server/core/test/test_config.cc index a0f22d75a..b2197a197 100644 --- a/server/core/test/test_config.cc +++ b/server/core/test/test_config.cc @@ -237,7 +237,7 @@ struct DISK_SPACE_THRESHOLD_TEST int dst_report(const DISK_SPACE_THRESHOLD_TEST& test, bool parsed, - MxsDiskSpaceThreshold& result) + SERVER::DiskSpaceLimits& result) { int nErrors = 0; @@ -360,7 +360,7 @@ int test_disk_space_threshold() { const DISK_SPACE_THRESHOLD_TEST& test = tests[i]; - MxsDiskSpaceThreshold dst; + SERVER::DiskSpaceLimits dst; bool parsed = config_parse_disk_space_threshold(&dst, test.zValue);