From 2b0eac2cd080c88d71ba73e86a0181bf242a488e Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Mon, 10 Dec 2018 16:51:35 +0200 Subject: [PATCH] Move server disk space threshold setting to private Server-class The setting can be read and written simultaneously and is protected with a mutex. The public SERVER-class is now abstract. --- include/maxscale/server.hh | 29 +++++++++++++++-- server/core/internal/server.hh | 31 ++++++++++++++++++- server/core/monitor.cc | 27 ++++++++-------- server/core/server.cc | 23 ++------------ server/core/test/test_local_address.cc | 5 ++- .../mariadbmon/test/test_cycle_find.cc | 5 +-- 6 files changed, 77 insertions(+), 43 deletions(-) diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index 44ca296f0..dcb7196e5 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -103,8 +103,9 @@ static uint64_t server_encode_version(const SERVER_VERSION* server_version) * the name of a protocol module that is loaded to implement the protocol * between the gateway and the server. */ -struct SERVER +class SERVER { +public: // Base settings char* name; /**< Server config name */ char address[MAX_SERVER_ADDRESS_LEN]; /**< Server hostname/IP-address */ @@ -151,7 +152,31 @@ struct SERVER * used * by rwsplit. TODO: Move to rwsplit */ bool warn_ssl_not_enabled;/**< SSL not used for an SSL enabled server */ - MxsDiskSpaceThreshold* disk_space_threshold;/**< Disk space thresholds */ + + virtual ~SERVER() + { + } + + /** + * Check if server has disk space threshold settings. + * + * @return True if limits exist + */ + virtual bool have_disk_space_limits() const = 0; + + /** + * Get a copy of disk space limit settings. + * + * @return A copy of settings + */ + virtual MxsDiskSpaceThreshold get_disk_space_limits() const = 0; + + /** + * Set new disk space limits for the server. + * + * @param new_limits New limits + */ + virtual void set_disk_space_limits(const MxsDiskSpaceThreshold& new_limits) = 0; }; /** diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index 90c38d660..4bb905ff7 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -36,6 +36,10 @@ public: { } + ~Server() override + { + } + int response_time_num_samples() const { return m_response_time.num_samples(); @@ -51,10 +55,35 @@ public: static void dprintServer(DCB*, const Server*); static void dprintPersistentDCBs(DCB*, const Server*); + bool have_disk_space_limits() const override + { + std::lock_guard guard(m_settings.lock); + return !m_settings.disk_space_limits.empty(); + } + + MxsDiskSpaceThreshold 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) override + { + std::lock_guard guard(m_settings.lock); + m_settings.disk_space_limits = new_limits; + } + mutable std::mutex m_lock; private: - maxbase::EMAverage m_response_time; + struct Settings + { + mutable std::mutex lock; /**< Protects array-like settings from concurrent access */ + MxsDiskSpaceThreshold disk_space_limits; /**< Disk space thresholds */ + }; + + maxbase::EMAverage m_response_time; /**< Response time calculations for this server */ + Settings m_settings; /**< Server settings */ }; void server_free(Server* server); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 07396af5c..49995a914 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -2623,12 +2623,11 @@ bool MonitorInstance::should_update_disk_space_status(const MXS_MONITORED_SERVER { bool should_check = false; - if (m_monitor->disk_space_check_interval - && (m_monitor->disk_space_threshold || pMs->server->disk_space_threshold) - && (pMs->disk_space_checked != -1)) // -1 means disabled + if ((m_monitor->disk_space_check_interval > 0) + && (pMs->disk_space_checked != -1) // -1 means disabled + && (m_monitor->disk_space_threshold || pMs->server->have_disk_space_limits())) { int64_t now = get_time_ms(); - if (now - pMs->disk_space_checked > m_monitor->disk_space_check_interval) { should_check = true; @@ -2674,21 +2673,21 @@ void MonitorInstance::update_disk_space_status(MXS_MONITORED_SERVER* pMs) if (rv == 0) { + // Server-specific setting takes precedence. + MxsDiskSpaceThreshold dst = pMs->server->get_disk_space_limits(); + if (dst.empty()) + { + dst = *m_monitor->disk_space_threshold; + } + bool disk_space_exhausted = false; - - MxsDiskSpaceThreshold* pDst = - pMs->server->disk_space_threshold ? - pMs->server->disk_space_threshold : m_monitor->disk_space_threshold; - mxb_assert(pDst); - int32_t star_max_percentage = -1; - std::set checked_paths; - for (auto i = pDst->begin(); i != pDst->end(); ++i) + for (const auto& dst_item : dst) { - string path = i->first; - int32_t max_percentage = i->second; + string path = dst_item.first; + int32_t max_percentage = dst_item.second; if (path == "*") { diff --git a/server/core/server.cc b/server/core/server.cc index f9084bfc7..28c39ff9d 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -182,7 +182,6 @@ SERVER* server_alloc(const char* name, MXS_CONFIG_PARAMETER* params) server->master_id = -1; server->master_err_is_logged = false; server->warn_ssl_not_enabled = true; - server->disk_space_threshold = NULL; if (*monuser && *monpw) { @@ -236,7 +235,6 @@ void server_free(Server* server) MXS_FREE(server->persistent); } - delete server->disk_space_threshold; delete server; } @@ -1513,29 +1511,12 @@ json_t* server_list_to_json(const char* host) bool server_set_disk_space_threshold(SERVER* server, const char* disk_space_threshold) { - bool rv = false; - MxsDiskSpaceThreshold dst; - - rv = config_parse_disk_space_threshold(&dst, disk_space_threshold); - + bool rv = config_parse_disk_space_threshold(&dst, disk_space_threshold); if (rv) { - if (!server->disk_space_threshold) - { - server->disk_space_threshold = new(std::nothrow) MxsDiskSpaceThreshold; - } - - if (server->disk_space_threshold) - { - server->disk_space_threshold->swap(dst); - } - else - { - rv = false; - } + server->set_disk_space_limits(dst); } - return rv; } diff --git a/server/core/test/test_local_address.cc b/server/core/test/test_local_address.cc index 01fcd3f47..e789a6698 100644 --- a/server/core/test/test_local_address.cc +++ b/server/core/test/test_local_address.cc @@ -16,6 +16,7 @@ #include #include #include +#include "../internal/server.hh" using namespace std; @@ -78,9 +79,7 @@ int test(bool success, const char* zHost, const char* zUser, const char* zPasswo MXS_CONFIG* config = config_get_global_options(); config->local_address = const_cast(zAddress); - SERVER server; - memset(&server, 0, sizeof(server)); - + Server server; strcpy(server.address, zHost); server.port = 3306; diff --git a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc index e206bb4dd..bc02f7b29 100644 --- a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc +++ b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc @@ -15,12 +15,13 @@ #include #include #include +#include #include #include #include #include #include -#include +#include "../../../../core/internal/server.hh" using std::string; using std::cout; @@ -155,7 +156,7 @@ void MariaDBMonitor::Test::init_servers(int count) for (int i = 1; i < count + 1; i++) { - SERVER* base_server = new SERVER; // Contents mostly undefined + auto base_server = new Server; // Contents mostly undefined string server_name = create_servername(i); base_server->name = MXS_STRDUP(server_name.c_str());