diff --git a/server/modules/monitor/mariadbmon/cluster_discovery.cc b/server/modules/monitor/mariadbmon/cluster_discovery.cc index 1fe6fb750..6d2e23951 100644 --- a/server/modules/monitor/mariadbmon/cluster_discovery.cc +++ b/server/modules/monitor/mariadbmon/cluster_discovery.cc @@ -166,7 +166,7 @@ void MariaDBMonitor::tarjan_scc_visit_node(MariaDBServer* node, */ void MariaDBMonitor::build_replication_graph() { - const bool use_hostnames = m_settings.shared.assume_unique_hostnames; + const bool use_hostnames = m_settings.assume_unique_hostnames; // First, reset all node data. for (MariaDBServer* server : m_servers) { @@ -174,12 +174,13 @@ void MariaDBMonitor::build_replication_graph() server->m_node.reset_results(); } - for (MariaDBServer* slave : m_servers) + for (auto slave : m_servers) { /* Check all slave connections of all servers. Connections are added even if one or both endpoints * are down or in maintenance. */ - for (SlaveStatus& slave_conn : slave->m_slave_status) + for (auto& slave_conn : slave->m_slave_status) { + slave_conn.master_server = nullptr; /* IF THIS PART IS CHANGED, CHANGE THE COMPARISON IN 'sstatus_arrays_topology_equal' * (in MariaDBServer) accordingly so that any possible topology changes are detected. */ if (slave_conn.slave_io_running != SlaveStatus::SLAVE_IO_NO && slave_conn.slave_sql_running) @@ -189,8 +190,7 @@ void MariaDBMonitor::build_replication_graph() bool is_external = false; if (use_hostnames) { - found_master = get_server(slave_conn.settings.master_endpoint.host(), - slave_conn.settings.master_endpoint.port()); + found_master = get_server(slave_conn.settings.master_endpoint); if (!found_master) { // Must be an external server. @@ -203,9 +203,11 @@ void MariaDBMonitor::build_replication_graph() * trust the "Master_Server_Id"-field of the SHOW SLAVE STATUS output if * the slave connection has been seen connected before. This means that * the graph will miss slave-master relations that have not connected - * while the monitor has been running. TODO: This data should be saved so - * that monitor restarts do not lose this information. */ - if (slave_conn.seen_connected) + * while the monitor has been running. + * + * TODO: This data should be saved so that monitor restarts do not lose + * this information. */ + if (slave_conn.master_server_id >= 0 && slave_conn.seen_connected) { // Valid slave connection, find the MariaDBServer with the matching server id. found_master = get_server(slave_conn.master_server_id); @@ -227,6 +229,7 @@ void MariaDBMonitor::build_replication_graph() * access later on. */ slave->m_node.parents.push_back(found_master); found_master->m_node.children.push_back(slave); + slave_conn.master_server = found_master; } else if (is_external) { diff --git a/server/modules/monitor/mariadbmon/mariadbmon.cc b/server/modules/monitor/mariadbmon/mariadbmon.cc index 9794e8dd1..50675a848 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.cc +++ b/server/modules/monitor/mariadbmon/mariadbmon.cc @@ -104,14 +104,15 @@ void MariaDBMonitor::reset_node_index_info() } } -MariaDBServer* MariaDBMonitor::get_server(const std::string& host, int port) +MariaDBServer* MariaDBMonitor::get_server(const EndPoint& search_ep) { // TODO: Do this with a map lookup + // TODO: Add DNS check here. MariaDBServer* found = NULL; for (MariaDBServer* server : m_servers) { - SERVER* srv = server->m_server_base->server; - if (host == srv->address && srv->port == port) + EndPoint srv(server->m_server_base->server); + if (srv == search_ep) { found = server; break; @@ -212,7 +213,7 @@ bool MariaDBMonitor::configure(const MXS_CONFIG_PARAMETER* params) m_settings.detect_stale_slave = params->get_bool("detect_stale_slave"); m_settings.detect_standalone_master = params->get_bool(CN_DETECT_STANDALONE_MASTER); m_settings.ignore_external_masters = params->get_bool("ignore_external_masters"); - m_settings.shared.assume_unique_hostnames = params->get_bool(CN_ASSUME_UNIQUE_HOSTNAMES); + m_settings.assume_unique_hostnames = params->get_bool(CN_ASSUME_UNIQUE_HOSTNAMES); m_settings.failcount = params->get_integer(CN_FAILCOUNT); m_settings.failover_timeout = params->get_duration(CN_FAILOVER_TIMEOUT).count(); m_settings.switchover_timeout = params->get_duration(CN_SWITCHOVER_TIMEOUT).count(); @@ -272,12 +273,12 @@ bool MariaDBMonitor::configure(const MXS_CONFIG_PARAMETER* params) } }; - warn_and_enable(&m_settings.shared.assume_unique_hostnames, CN_ASSUME_UNIQUE_HOSTNAMES); + warn_and_enable(&m_settings.assume_unique_hostnames, CN_ASSUME_UNIQUE_HOSTNAMES); warn_and_enable(&m_settings.auto_failover, CN_AUTO_FAILOVER); warn_and_enable(&m_settings.auto_rejoin, CN_AUTO_REJOIN); } - if (!m_settings.shared.assume_unique_hostnames) + if (!m_settings.assume_unique_hostnames) { const char requires[] = "%s requires that %s is on."; if (m_settings.auto_failover) diff --git a/server/modules/monitor/mariadbmon/mariadbmon.hh b/server/modules/monitor/mariadbmon/mariadbmon.hh index 6dc2e46ff..882247db4 100644 --- a/server/modules/monitor/mariadbmon/mariadbmon.hh +++ b/server/modules/monitor/mariadbmon/mariadbmon.hh @@ -221,6 +221,8 @@ private: * TODO: think about removing */ bool ignore_external_masters {false}; /* Ignore masters outside of the monitor configuration. * TODO: requires work */ + bool assume_unique_hostnames {true}; /* Are server hostnames consistent between MaxScale and + * servers */ int failcount {1}; /* Number of ticks master must be down before it's considered * totally down, allowing failover or master change. */ @@ -264,7 +266,7 @@ private: std::string diagnostics_to_string() const; json_t* to_json() const; - MariaDBServer* get_server(const std::string& host, int port); + MariaDBServer* get_server(const EndPoint& search_ep); MariaDBServer* get_server(int64_t id); MariaDBServer* get_server(mxs::MonitorServer* mon_server); MariaDBServer* get_server(SERVER* server); diff --git a/server/modules/monitor/mariadbmon/mariadbserver.cc b/server/modules/monitor/mariadbmon/mariadbserver.cc index 218d477ce..247f22e20 100644 --- a/server/modules/monitor/mariadbmon/mariadbserver.cc +++ b/server/modules/monitor/mariadbmon/mariadbserver.cc @@ -369,6 +369,10 @@ bool MariaDBServer::do_show_slave_status(string* errmsg_out) { new_row.last_data_time = old_row->last_data_time; } + + // Copy master server pointer from old row. If this line is not reached because old row does + // not exist, then the topology rebuild will set the master pointer. + new_row.master_server = old_row->master_server; } // Finally, set the connection status. @@ -1004,6 +1008,7 @@ bool MariaDBServer::sstatus_array_topology_equal(const SlaveStatusArray& new_sla if (new_row.slave_io_running != old_row.slave_io_running || new_row.slave_sql_running != old_row.slave_sql_running || new_row.settings.master_endpoint != old_row.settings.master_endpoint + || new_row.settings.name != old_row.settings.name || new_row.master_server_id != old_row.master_server_id) { rval = false; @@ -1176,39 +1181,16 @@ bool MariaDBServer::can_be_promoted(OperationType op, const MariaDBServer* demot const SlaveStatus* MariaDBServer::slave_connection_status(const MariaDBServer* target) const { + mxb_assert(target); // The slave node may have several slave connections, need to find the one that is - // connected to the parent. TODO: Use the information gathered in 'build_replication_graph' - // to skip this function, as the contents are very similar. + // connected to the parent. Most of this has already been done in 'build_replication_graph'. const SlaveStatus* rval = NULL; - if (m_settings.assume_unique_hostnames) + for (const SlaveStatus& ss : m_slave_status) { - // Can simply compare host:port. - EndPoint target_endpoint(target->m_server_base->server); - for (const SlaveStatus& ss : m_slave_status) + if (ss.master_server == target) { - if (ss.settings.master_endpoint == target_endpoint && ss.slave_sql_running - && ss.slave_io_running != SlaveStatus::SLAVE_IO_NO) - { - rval = &ss; - break; - } - } - } - else - { - // Compare server id:s instead. If the master's id is wrong (e.g. never updated) this gives a - // wrong result. Also gives wrong result if monitor has never seen the slave connection in the - // connected state. - auto target_id = target->m_server_id; - for (const SlaveStatus& ss : m_slave_status) - { - auto master_id = ss.master_server_id; - if (master_id > 0 && master_id == target_id && ss.slave_sql_running && ss.seen_connected - && ss.slave_io_running != SlaveStatus::SLAVE_IO_NO) - { - rval = &ss; - break; - } + rval = &ss; + break; } } return rval; diff --git a/server/modules/monitor/mariadbmon/server_utils.hh b/server/modules/monitor/mariadbmon/server_utils.hh index 1bf9f455c..3eaf5a3f0 100644 --- a/server/modules/monitor/mariadbmon/server_utils.hh +++ b/server/modules/monitor/mariadbmon/server_utils.hh @@ -239,8 +239,10 @@ public: Settings settings; /* User-defined settings for the slave connection. */ - bool seen_connected = false; /* Has this slave connection been seen connected, - * meaning that the master server id is correct? */ + /* If the master is a monitored server, it's written here. */ + const MariaDBServer* master_server {nullptr}; + /* Has this slave connection been seen connected, meaning that the master server id is correct? */ + bool seen_connected = false; int64_t master_server_id = SERVER_ID_UNKNOWN; /* The master's server_id value. Valid ids are * 32bit unsigned. -1 is unread/error. */ @@ -320,8 +322,4 @@ public: /* Should failover/switchover enable/disable any scheduled events on the servers during * promotion/demotion? */ bool handle_event_scheduler {true}; - - // Miscellaneous settings - bool assume_unique_hostnames {true}; /**< Are server hostnames consistent between MaxScale and - * servers */ -}; \ No newline at end of file +}; diff --git a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc index 03e3c6146..ad642adbb 100644 --- a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc +++ b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc @@ -152,7 +152,7 @@ int MariaDBMonitor::Test::run_tests() void MariaDBMonitor::Test::init_servers(int count) { clear_servers(); - m_monitor->m_settings.shared.assume_unique_hostnames = m_use_hostnames; + m_monitor->m_settings.assume_unique_hostnames = m_use_hostnames; mxb_assert(m_monitor->m_servers.empty() && m_monitor->m_servers_by_id.empty()); for (int i = 1; i < count + 1; i++) @@ -307,7 +307,7 @@ int MariaDBMonitor::Test::check_result_cycles(CycleArray expected_cycles) MariaDBServer* MariaDBMonitor::Test::get_server(int i) { - auto rval = m_use_hostnames ? m_monitor->get_server(create_hostname(i), i) : + auto rval = m_use_hostnames ? m_monitor->get_server(EndPoint(create_hostname(i), i)) : m_monitor->get_server(i); mxb_assert(rval); return rval;