Cleanup slave status handling

Further reduce direct indexing slave status array to improve compatibility with
multimaster replication.

server/modules/monitor/mariadbmon/cluster_manipulation.cc

@@ -1251,9 +1251,10 @@ MariaDBServer* MariaDBMonitor::select_promotion_target(MariaDBServer* demotion_t
 
     // Check which candidate is best
     string current_best_reason;
+    int64_t gtid_domain = m_master_gtid_domain;
     for (MariaDBServer* cand : candidates)
     {
-        if (is_candidate_better(cand, current_best, m_master_gtid_domain, &current_best_reason))
+        if (is_candidate_better(cand, current_best, demotion_target, gtid_domain, &current_best_reason))
         {
             // Select the server for promotion, for now.
             current_best = cand;
@@ -1263,9 +1264,9 @@ MariaDBServer* MariaDBMonitor::select_promotion_target(MariaDBServer* demotion_t
     // Check if any of the excluded servers would be better than the best candidate. Only print one item.
     if (log_mode == Log::ON)
     {
-        for (MariaDBServer* excluded_info : valid_but_excluded)
+        for (MariaDBServer* excluded : valid_but_excluded)
         {
-            const char* excluded_name = excluded_info->name();
+            const char* excluded_name = excluded->name();
             if (current_best == NULL)
             {
                 const char EXCLUDED_ONLY_CAND[] = "Server '%s' is a viable choice for new master, "
@@ -1273,7 +1274,7 @@ MariaDBServer* MariaDBMonitor::select_promotion_target(MariaDBServer* demotion_t
                 MXS_WARNING(EXCLUDED_ONLY_CAND, excluded_name);
                 break;
             }
-            else if (is_candidate_better(excluded_info, current_best, m_master_gtid_domain))
+            else if (is_candidate_better(excluded, current_best, demotion_target, gtid_domain))
             {
                 // Print a warning if this server is actually a better candidate than the previous best.
                 const char EXCLUDED_CAND[] = "Server '%s' is superior to current best candidate '%s', "
@@ -1319,19 +1320,25 @@ bool MariaDBMonitor::server_is_excluded(const MariaDBServer* server)
  *
  * @param candidate_info Server info of new candidate
  * @param current_best_info Server info of current best choice
+ * @param demotion_target Server which will be demoted
  * @param gtid_domain Which domain to compare
  * @param reason_out Why is the candidate better than current_best
  * @return True if candidate is better
  */
 bool MariaDBMonitor::is_candidate_better(const MariaDBServer* candidate,
                                          const MariaDBServer* current_best,
+                                         const MariaDBServer* demotion_target,
                                          uint32_t gtid_domain,
                                          std::string* reason_out)
 {
+    const SlaveStatus* cand_slave_conn = candidate->slave_connection_status(demotion_target);
+    const SlaveStatus* curr_best_slave_conn = current_best->slave_connection_status(demotion_target);
+    mxb_assert(cand_slave_conn && curr_best_slave_conn);
+
+    uint64_t cand_io = cand_slave_conn->gtid_io_pos.get_gtid(gtid_domain).m_sequence;
+    uint64_t curr_io = curr_best_slave_conn->gtid_io_pos.get_gtid(gtid_domain).m_sequence;
     string reason;
     bool is_better = false;
-    uint64_t cand_io = candidate->m_slave_status[0].gtid_io_pos.get_gtid(gtid_domain).m_sequence;
-    uint64_t curr_io = current_best->m_slave_status[0].gtid_io_pos.get_gtid(gtid_domain).m_sequence;
     // A slave with a later event in relay log is always preferred.
     if (cand_io > curr_io)
     {
@@ -1597,7 +1604,7 @@ void MariaDBMonitor::check_cluster_operations_support()
                                             server->name());
                 printer.cat(all_reasons, reason);
             }
-            if (!server->uses_gtid())
+            else if (server->m_slave_status[0].gtid_io_pos.empty())
             {
                 supported = false;
                 auto reason = string_printf("Server '%s' is not using gtid-replication.", server->name());

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -603,12 +603,14 @@ void MariaDBMonitor::update_external_master()
 {
     if (m_master->is_slave_of_ext_master())
     {
-        mxb_assert(!m_master->m_slave_status.empty());
+        mxb_assert(!m_master->m_slave_status.empty() && !m_master->m_node.external_masters.empty());
-        if (m_master->m_slave_status[0].master_host != m_external_master_host
+        // TODO: Add support for multiple external masters.
-            || m_master->m_slave_status[0].master_port != m_external_master_port)
+        auto& master_sstatus = m_master->m_slave_status[0];
+        if (master_sstatus.master_host != m_external_master_host ||
+            master_sstatus.master_port != m_external_master_port)
         {
-            const string new_ext_host = m_master->m_slave_status[0].master_host;
+            const string new_ext_host = master_sstatus.master_host;
-            const int new_ext_port = m_master->m_slave_status[0].master_port;
+            const int new_ext_port = master_sstatus.master_port;
             if (m_external_master_port == PORT_UNKNOWN)
             {
                 MXS_NOTICE("Cluster master server is replicating from an external master: %s:%d",

server/modules/monitor/mariadbmon/mariadbmon.hh

@@ -281,6 +281,7 @@ private:
     bool server_is_excluded(const MariaDBServer* server);
     bool is_candidate_better(const MariaDBServer* candidate,
                              const MariaDBServer* current_best,
+                             const MariaDBServer* demotion_target,
                              uint32_t gtid_domain,
                              std::string* reason_out = NULL);
     bool promote_new_master(MariaDBServer* new_master, json_t** err_out);

server/modules/monitor/mariadbmon/mariadbserver.cc

@@ -601,16 +601,6 @@ json_t* MariaDBServer::to_json() const
     return result;
 }
 
-bool MariaDBServer::uses_gtid(std::string* error_out)
-{
-    bool using_gtid = !m_slave_status.empty() && !m_slave_status[0].gtid_io_pos.empty();
-    if (!using_gtid && error_out)
-    {
-        *error_out = string_printf("Server '%s' is not using gtid replication.", name());
-    }
-    return using_gtid;
-}
-
 bool MariaDBServer::can_replicate_from(MariaDBServer* master, string* error_out)
 {
     bool rval = false;
@@ -1085,7 +1075,7 @@ bool MariaDBServer::can_be_promoted(ClusterOperation op,
     return promotable;
 }
 
-const SlaveStatus* MariaDBServer::slave_connection_status(const MariaDBServer* target)
+const SlaveStatus* MariaDBServer::slave_connection_status(const MariaDBServer* target) const
 {
     // The slave node may have several slave connections, need to find the one that is
     // connected to the parent. This section is quite similar to the one in

server/modules/monitor/mariadbmon/mariadbserver.hh

@@ -274,7 +274,7 @@ public:
      * @param target Immediate master or relay server
      * @return The slave status info of the slave thread, or NULL if not found or not accepted
      */
-    const SlaveStatus* slave_connection_status(const MariaDBServer* target);
+    const SlaveStatus* slave_connection_status(const MariaDBServer* target) const;
 
     /**
      * Is binary log on? 'update_replication_settings' should be ran before this function to query the data.
@@ -384,15 +384,6 @@ public:
      */
     std::string diagnostics() const;
 
-    /**
-     * Check if server is using gtid replication.
-     *
-     * @param error_out Error output
-     * @return True if using gtid-replication. False if not, or if server is not a slave or otherwise does
-     * not have a gtid_IO_Pos.
-     */
-    bool uses_gtid(std::string* error_out = NULL);
-
     /**
      * Checks if this server can replicate from master. Only considers gtid:s and only detects obvious errors.
      * The non-detected errors will mostly be detected once the slave tries to start replicating.