From cef4e836bc1d1cf0806359a47b22b8e29bcaeee8 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Thu, 24 Jan 2019 12:43:27 +0200 Subject: [PATCH] MXS-2271 Store monitored servers in a vector The array is still a public member because it's used in several non-member functions. --- include/maxscale/monitor.hh | 19 +-- server/core/config_runtime.cc | 4 +- server/core/monitor.cc | 115 +++++++----------- .../monitor/clustrixmon/clustrixmonitor.cc | 63 +++++----- server/modules/monitor/galeramon/galeramon.cc | 14 +-- .../modules/monitor/mariadbmon/mariadbmon.cc | 5 +- server/modules/monitor/mmmon/mmmon.cc | 9 +- 7 files changed, 81 insertions(+), 148 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index f425a7327..2e6356046 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -138,25 +138,8 @@ public: uint64_t mon_prev_status = -1; /**< Status before starting the current monitor loop */ uint64_t pending_status = 0; /**< Status during current monitor loop */ int64_t disk_space_checked = 0; /**< When was the disk space checked the last time */ - - MXS_MONITORED_SERVER* next = nullptr; /**< The next server in the list */ }; -namespace std -{ - -inline mxb::intrusive_slist_iterator begin(MXS_MONITORED_SERVER& monitored_server) -{ - return mxb::intrusive_slist_iterator(monitored_server); -} - -inline mxb::intrusive_slist_iterator end(MXS_MONITORED_SERVER& monitored_server) -{ - return mxb::intrusive_slist_iterator(); -} - -} - #define MAX_MONITOR_USER_LEN 512 #define MAX_MONITOR_PASSWORD_LEN 512 @@ -228,7 +211,7 @@ public: uint8_t journal_hash[SHA_DIGEST_LENGTH]; /**< SHA1 hash of the latest written journal */ MXS_CONFIG_PARAMETER* parameters = nullptr; /**< Configuration parameters */ - MXS_MONITORED_SERVER* monitored_servers = nullptr; /**< List of servers the monitor monitors */ + std::vector m_servers; /**< Monitored servers */ char user[MAX_MONITOR_USER_LEN]; /**< Monitor username */ char password[MAX_MONITOR_PASSWORD_LEN]; /**< Monitor password */ diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 0913171f9..34ea47890 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -1422,9 +1422,9 @@ bool runtime_destroy_monitor(Monitor* monitor) { monitor_stop(monitor); - while (monitor->monitored_servers) + while (!monitor->m_servers.empty()) { - monitor_remove_server(monitor, monitor->monitored_servers->server); + monitor_remove_server(monitor, monitor->m_servers[0]->server); } monitor_deactivate(monitor); MXS_NOTICE("Destroyed monitor '%s'", monitor->name); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index bb419beb0..5b16115b5 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -137,7 +137,7 @@ ThisUnit this_unit; } -static void monitor_server_free_all(MXS_MONITORED_SERVER* servers); +static void monitor_server_free_all(std::vector& servers); static void remove_server_journal(Monitor* monitor); static bool journal_is_stale(Monitor* monitor, time_t max_age); static const char* monitor_state_to_string(monitor_state_t state); @@ -230,7 +230,7 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) Monitor::~Monitor() { config_parameter_free(parameters); - monitor_server_free_all(monitored_servers); + monitor_server_free_all(m_servers); MXS_FREE((const_cast(name))); } @@ -308,13 +308,11 @@ void monitor_stop(Monitor* monitor) monitor->stop(); monitor->state = MONITOR_STATE_STOPPED; - MXS_MONITORED_SERVER* db = monitor->monitored_servers; - while (db) + for (auto db : monitor->m_servers) { // TODO: Create a generic entry point for this or move it inside stopMonitor mysql_close(db->con); db->con = NULL; - db = db->next; } } } @@ -372,19 +370,7 @@ bool monitor_add_server(Monitor* mon, SERVER* server) { Guard guard(mon->lock); - if (mon->monitored_servers == NULL) - { - mon->monitored_servers = db; - } - else - { - MXS_MONITORED_SERVER* ptr = mon->monitored_servers; - while (ptr->next != NULL) - { - ptr = ptr->next; - } - ptr->next = db; - } + mon->m_servers.push_back(db); } if (old_state == MONITOR_STATE_RUNNING) @@ -412,14 +398,13 @@ static void monitor_server_free(MXS_MONITORED_SERVER* tofree) * Free monitor server list * @param servers Servers to free */ -static void monitor_server_free_all(MXS_MONITORED_SERVER* servers) +static void monitor_server_free_all(std::vector& servers) { - while (servers) + for (auto server : servers) { - MXS_MONITORED_SERVER* tofree = servers; - servers = servers->next; - monitor_server_free(tofree); + monitor_server_free(server); } + servers.clear(); } /** @@ -437,26 +422,16 @@ void monitor_remove_server(Monitor* mon, SERVER* server) monitor_stop(mon); } - MXS_MONITORED_SERVER* ptr = mon->monitored_servers; + MXS_MONITORED_SERVER* ptr = nullptr; { Guard guard(mon->lock); - - if (ptr && ptr->server == server) + for (auto iter = mon->m_servers.begin(); iter != mon->m_servers.end(); ++iter) { - mon->monitored_servers = mon->monitored_servers->next; - } - else - { - MXS_MONITORED_SERVER* prev = ptr; - while (ptr) + if ((*iter)->server == server) { - if (ptr->server == server) - { - prev->next = ptr->next; - break; - } - prev = ptr; - ptr = ptr->next; + ptr = *iter; + mon->m_servers.erase(iter); + break; } } } @@ -535,7 +510,7 @@ void Monitor::show(DCB* dcb) const char* sep = ""; - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { dcb_printf(dcb, "%s[%s]:%d", sep, db->server->address, db->server->port); sep = ", "; @@ -710,8 +685,7 @@ std::unique_ptr monitor_get_list() bool Monitor::test_permissions(const string& query) { auto monitor = this; - if (monitor->monitored_servers == NULL // No servers to check - || config_get_global_options()->skip_permission_checks) + if (monitor->m_servers.empty() || config_get_global_options()->skip_permission_checks) { return true; } @@ -720,7 +694,7 @@ bool Monitor::test_permissions(const string& query) char* dpasswd = decrypt_password(monitor->password); bool rval = false; - for (MXS_MONITORED_SERVER* mondb = monitor->monitored_servers; mondb; mondb = mondb->next) + for (MXS_MONITORED_SERVER* mondb : monitor->m_servers) { if (!mon_connection_is_ok(mon_ping_or_connect_to_db(monitor, mondb))) { @@ -1060,16 +1034,14 @@ static void mon_append_node_names(Monitor* mon, int status, credentials_approach_t approach = CREDENTIALS_EXCLUDE) { - MXS_MONITORED_SERVER* servers = mon->monitored_servers; - const char* separator = ""; // Some extra space for port and separator char arr[SERVER::MAX_MONUSER_LEN + SERVER::MAX_MONPW_LEN + SERVER::MAX_ADDRESS_LEN + 64]; dest[0] = '\0'; - while (servers && len) + for (auto iter = mon->m_servers.begin(); iter != mon->m_servers.end() && len; ++iter) { - Server* server = static_cast(servers->server); + Server* server = static_cast((*iter)->server); if (status == 0 || server->status & status) { if (approach == CREDENTIALS_EXCLUDE) @@ -1111,7 +1083,6 @@ static void mon_append_node_names(Monitor* mon, len -= arrlen; } } - servers = servers->next; } } @@ -1159,14 +1130,14 @@ bool mon_print_fail_status(MXS_MONITORED_SERVER* mon_srv) return mon_srv->server->is_down() && mon_srv->mon_err_count == 0; } -static MXS_MONITORED_SERVER* find_parent_node(MXS_MONITORED_SERVER* servers, +static MXS_MONITORED_SERVER* find_parent_node(const std::vector& servers, MXS_MONITORED_SERVER* target) { MXS_MONITORED_SERVER* rval = NULL; if (target->server->master_id > 0) { - for (MXS_MONITORED_SERVER* node = servers; node; node = node->next) + for (MXS_MONITORED_SERVER* node : servers) { if (node->server->node_id == target->server->master_id) { @@ -1179,7 +1150,7 @@ static MXS_MONITORED_SERVER* find_parent_node(MXS_MONITORED_SERVER* servers, return rval; } -static std::string child_nodes(MXS_MONITORED_SERVER* servers, +static std::string child_nodes(const std::vector& servers, MXS_MONITORED_SERVER* parent) { std::stringstream ss; @@ -1188,7 +1159,7 @@ static std::string child_nodes(MXS_MONITORED_SERVER* servers, { bool have_content = false; - for (MXS_MONITORED_SERVER* node = servers; node; node = node->next) + for (MXS_MONITORED_SERVER* node : servers) { if (node->server->master_id == parent->server->node_id) { @@ -1218,7 +1189,7 @@ int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* c if (externcmd_matches(cmd, "$PARENT")) { std::stringstream ss; - MXS_MONITORED_SERVER* parent = find_parent_node(mon->monitored_servers, ptr); + MXS_MONITORED_SERVER* parent = find_parent_node(mon->m_servers, ptr); if (parent) { @@ -1229,7 +1200,7 @@ int monitor_launch_command(Monitor* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* c if (externcmd_matches(cmd, "$CHILDREN")) { - externcmd_substitute_arg(cmd, "[$]CHILDREN", child_nodes(mon->monitored_servers, ptr).c_str()); + externcmd_substitute_arg(cmd, "[$]CHILDREN", child_nodes(mon->m_servers, ptr).c_str()); } if (externcmd_matches(cmd, "$EVENT")) @@ -1484,7 +1455,7 @@ Monitor* monitor_server_in_use(const SERVER* server) Guard guard(monitor->lock); if (monitor->active) { - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { if (db->server == server) { @@ -1517,12 +1488,12 @@ static bool create_monitor_config(const Monitor* monitor, const char* filename) dprintf(file, "[%s]\n", monitor->name); dprintf(file, "%s=monitor\n", CN_TYPE); - if (monitor->monitored_servers) + if (!monitor->m_servers.empty()) { dprintf(file, "%s=", CN_SERVERS); - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { - if (db != monitor->monitored_servers) + if (db != monitor->m_servers[0]) { dprintf(file, ","); } @@ -1589,7 +1560,7 @@ bool monitor_serialize(const Monitor* monitor) void mon_hangup_failed_servers(Monitor* monitor) { - for (MXS_MONITORED_SERVER* ptr = monitor->monitored_servers; ptr; ptr = ptr->next) + for (MXS_MONITORED_SERVER* ptr : monitor->m_servers) { if (mon_status_changed(ptr) && (!(ptr->server->is_usable()) || !(ptr->server->is_in_cluster()))) { @@ -1621,8 +1592,7 @@ void monitor_check_maintenance_requests(Monitor* monitor) SERVER::MAINTENANCE_FLAG_NOCHECK); if (flags_changed != SERVER::MAINTENANCE_FLAG_NOCHECK) { - MXS_MONITORED_SERVER* ptr = monitor->monitored_servers; - while (ptr) + for (auto ptr : monitor->m_servers) { // The only server status bit the admin may change is the [Maintenance] bit. int admin_msg = atomic_exchange_int(&ptr->server->maint_request, SERVER::MAINTENANCE_NO_CHANGE); @@ -1635,7 +1605,6 @@ void monitor_check_maintenance_requests(Monitor* monitor) { ptr->server->clear_status(SERVER_MAINT); } - ptr = ptr->next; } } } @@ -1645,7 +1614,7 @@ void mon_process_state_changes(Monitor* monitor, const char* script, uint64_t ev bool master_down = false; bool master_up = false; - for (MXS_MONITORED_SERVER* ptr = monitor->monitored_servers; ptr; ptr = ptr->next) + for (MXS_MONITORED_SERVER* ptr : monitor->m_servers) { if (mon_status_changed(ptr)) { @@ -1742,10 +1711,10 @@ json_t* monitor_json_data(const Monitor* monitor, const char* host) } } - if (monitor->monitored_servers) + if (!monitor->m_servers.empty()) { json_t* mon_rel = mxs_json_relationship(host, MXS_JSON_API_SERVERS); - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { mxs_json_add_relation(mon_rel, db->server->name(), CN_SERVERS); } @@ -1792,7 +1761,7 @@ json_t* monitor_relations_to_server(const SERVER* server, const char* host) Guard guard(mon->lock); if (mon->active) { - for (MXS_MONITORED_SERVER* db = mon->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : mon->m_servers) { if (db->server == server) { @@ -1906,7 +1875,7 @@ static void store_data(Monitor* monitor, MXS_MONITORED_SERVER* master, uint8_t* *ptr++ = MMB_SCHEMA_VERSION; /** Store the states of all servers */ - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { *ptr++ = (char)SVT_SERVER; // Value type memcpy(ptr, db->server->name(), strlen(db->server->name()));// Name of the server @@ -1995,7 +1964,7 @@ static bool has_null_terminator(const char* data, const char* end) */ static const char* process_server(Monitor* monitor, const char* data, const char* end) { - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { if (strcmp(db->server->name(), data) == 0) { @@ -2026,7 +1995,7 @@ static const char* process_master(Monitor* monitor, { if (master) { - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { if (strcmp(db->server->name(), data) == 0) { @@ -2102,7 +2071,7 @@ void store_server_journal(Monitor* monitor, MXS_MONITORED_SERVER* master) /** Calculate how much memory we need to allocate */ uint32_t size = MMB_LEN_SCHEMA_VERSION + MMB_LEN_CRC32; - for (MXS_MONITORED_SERVER* db = monitor->monitored_servers; db; db = db->next) + for (MXS_MONITORED_SERVER* db : monitor->m_servers) { /** Each server is stored as a type byte and a null-terminated string * followed by eight byte server status. */ @@ -2300,7 +2269,7 @@ static bool journal_is_stale(Monitor* monitor, time_t max_age) MXS_MONITORED_SERVER* mon_get_monitored_server(const Monitor* mon, SERVER* search_server) { mxb_assert(mon && search_server); - for (MXS_MONITORED_SERVER* iter = mon->monitored_servers; iter != NULL; iter = iter->next) + for (MXS_MONITORED_SERVER* iter : mon->m_servers) { if (iter->server == search_server) { @@ -2671,7 +2640,7 @@ bool MonitorWorker::has_sufficient_permissions() void MonitorWorker::flush_server_status() { - for (MXS_MONITORED_SERVER* pMs = m_monitor->monitored_servers; pMs; pMs = pMs->next) + for (MXS_MONITORED_SERVER* pMs : m_servers) { if (!pMs->server->is_in_maint()) { @@ -2692,7 +2661,7 @@ void MonitorWorkerSimple::tick() { pre_tick(); - for (MXS_MONITORED_SERVER* pMs = m_monitor->monitored_servers; pMs; pMs = pMs->next) + for (MXS_MONITORED_SERVER* pMs : m_servers) { if (!pMs->server->is_in_maint()) { diff --git a/server/modules/monitor/clustrixmon/clustrixmonitor.cc b/server/modules/monitor/clustrixmon/clustrixmonitor.cc index d08df43c2..3c34cef90 100644 --- a/server/modules/monitor/clustrixmon/clustrixmonitor.cc +++ b/server/modules/monitor/clustrixmon/clustrixmonitor.cc @@ -129,12 +129,9 @@ void ClustrixMonitor::choose_hub() // If that fails, then we check the bootstrap servers, but only if // it was not checked above. - auto b = begin(*(this->monitored_servers)); - auto e = end(*(this->monitored_servers)); - - for (auto it = b; !pHub_con && (it != e); ++it) + for (auto it = m_servers.begin(); !pHub_con && (it != m_servers.end()); ++it) { - MXS_MONITORED_SERVER& ms = *it; + MXS_MONITORED_SERVER& ms = **it; if (ips.find(ms.server->address) == ips.end()) { @@ -444,40 +441,36 @@ bool ClustrixMonitor::check_cluster_membership(std::map void ClustrixMonitor::update_server_statuses() { - mxb_assert(this->monitored_servers); + mxb_assert(!m_servers.empty()); - auto b = std::begin(*this->monitored_servers); - auto e = std::end(*this->monitored_servers); + for (auto ms : m_servers) + { + monitor_stash_current_status(ms); - for_each(b, e, - [this](MXS_MONITORED_SERVER& ms) { - monitor_stash_current_status(&ms); + auto it = find_if(m_nodes.begin(), m_nodes.end(), + [ms](const std::pair& element) -> bool { + const ClustrixNode& info = element.second; + return ms->server->address == info.ip(); + }); - auto it = find_if(m_nodes.begin(), m_nodes.end(), - [&ms](const std::pair& element) -> bool { - const ClustrixNode& info = element.second; - return ms.server->address == info.ip(); - }); + if (it != m_nodes.end()) + { + const ClustrixNode& info = it->second; - if (it != m_nodes.end()) - { - const ClustrixNode& info = it->second; - - if (info.is_running()) - { - monitor_set_pending_status(&ms, SERVER_RUNNING); - } - else - { - monitor_clear_pending_status(&ms, SERVER_RUNNING); - } - } - else - { - monitor_clear_pending_status(&ms, SERVER_RUNNING); - } - - }); + if (info.is_running()) + { + monitor_set_pending_status(ms, SERVER_RUNNING); + } + else + { + monitor_clear_pending_status(ms, SERVER_RUNNING); + } + } + else + { + monitor_clear_pending_status(ms, SERVER_RUNNING); + } + } } void ClustrixMonitor::make_health_check() diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index 6ddef5450..ef3fd1498 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/server/modules/monitor/galeramon/galeramon.cc @@ -251,9 +251,7 @@ void GaleraMonitor::post_tick() m_master = set_cluster_master(m_master, candidate_master, m_disableMasterFailback); - MXS_MONITORED_SERVER* ptr = m_monitor->monitored_servers; - - while (ptr) + for (auto ptr : m_servers) { const int repl_bits = (SERVER_SLAVE | SERVER_MASTER | SERVER_MASTER_STICKINESS); if ((ptr->pending_status & SERVER_JOINED) && !m_disableMasterRoleSetting) @@ -288,7 +286,6 @@ void GaleraMonitor::post_tick() monitor_clear_pending_status(ptr, repl_bits); monitor_set_pending_status(ptr, 0); } - ptr = ptr->next; } if (is_cluster == 0 && m_log_no_members) @@ -363,13 +360,12 @@ static bool using_xtrabackup(MXS_MONITORED_SERVER* database, const char* server_ */ MXS_MONITORED_SERVER* GaleraMonitor::get_candidate_master() { - MXS_MONITORED_SERVER* moitor_servers = m_monitor->monitored_servers; MXS_MONITORED_SERVER* candidate_master = NULL; long min_id = -1; int minval = INT_MAX; int currval; /* set min_id to the lowest value of moitor_servers->server->node_id */ - while (moitor_servers) + for (auto moitor_servers : m_servers) { if (!moitor_servers->server->is_in_maint() && (moitor_servers->pending_status & SERVER_JOINED)) @@ -402,7 +398,6 @@ MXS_MONITORED_SERVER* GaleraMonitor::get_candidate_master() } } } - moitor_servers = moitor_servers->next; } if (!m_use_priority && !m_disableMasterFailback @@ -513,10 +508,8 @@ void GaleraMonitor::update_sst_donor_nodes(int is_cluster) strcpy(donor_list, DONOR_LIST_SET_VAR); - ptr = m_monitor->monitored_servers; - /* Create an array of slave nodes */ - while (ptr) + for (auto ptr : m_servers) { if ((ptr->pending_status & SERVER_JOINED) && (ptr->pending_status & SERVER_SLAVE)) { @@ -533,7 +526,6 @@ void GaleraMonitor::update_sst_donor_nodes(int is_cluster) ignore_priority = false; } } - ptr = ptr->next; } /* Set order type */ diff --git a/server/modules/monitor/mariadbmon/mariadbmon.cc b/server/modules/monitor/mariadbmon/mariadbmon.cc index 9d84ece56..af18e8bfe 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.cc +++ b/server/modules/monitor/mariadbmon/mariadbmon.cc @@ -78,7 +78,7 @@ void MariaDBMonitor::reset_server_info() clear_server_info(); // Next, initialize the data. - for (auto mon_server = m_monitor->monitored_servers; mon_server; mon_server = mon_server->next) + for (auto mon_server : Monitor::m_servers) { m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(), m_assume_unique_hostnames)); } @@ -433,8 +433,9 @@ void MariaDBMonitor::tick() { /* Update MXS_MONITORED_SERVER->pending_status. This is where the monitor loop writes it's findings. * Also, backup current status so that it can be compared to any deduced state. */ - for (auto mon_srv = m_monitor->monitored_servers; mon_srv; mon_srv = mon_srv->next) + for (auto srv : m_servers) { + auto mon_srv = srv->m_server_base; auto status = mon_srv->server->status; mon_srv->pending_status = status; mon_srv->mon_prev_status = status; diff --git a/server/modules/monitor/mmmon/mmmon.cc b/server/modules/monitor/mmmon/mmmon.cc index fc794a21a..739a8cd25 100644 --- a/server/modules/monitor/mmmon/mmmon.cc +++ b/server/modules/monitor/mmmon/mmmon.cc @@ -322,7 +322,7 @@ void MMMonitor::post_tick() /* Update server status from monitor pending status on that server*/ - for (MXS_MONITORED_SERVER* ptr = m_monitor->monitored_servers; ptr; ptr = ptr->next) + for (MXS_MONITORED_SERVER* ptr : m_servers) { if (!ptr->server->is_in_maint()) { @@ -367,9 +367,7 @@ MXS_MONITORED_SERVER* MMMonitor::get_current_master() Monitor* mon = m_monitor; MXS_MONITORED_SERVER* ptr; - ptr = mon->monitored_servers; - - while (ptr) + for (auto ptr : m_servers) { /* The server could be in SERVER_IN_MAINT * that means SERVER_IS_RUNNING returns 0 @@ -377,7 +375,6 @@ MXS_MONITORED_SERVER* MMMonitor::get_current_master() */ if (ptr->server->is_down()) { - ptr = ptr->next; continue; } @@ -385,8 +382,6 @@ MXS_MONITORED_SERVER* MMMonitor::get_current_master() { m_master = ptr; } - - ptr = ptr->next; }