MXS-2313: Combine slave selection functions

The functions now do the candidate selection in one go. This removes the
unnecessary copying and split logic of server selection.

server/modules/routing/readwritesplit/readwritesplit.hh

@@ -399,21 +399,6 @@ mxs::RWBackend* get_root_master(const mxs::PRWBackends& backends, mxs::RWBackend
  */
 std::pair<int, int> get_slave_counts(mxs::PRWBackends& backends, mxs::RWBackend* master);
 
-/**
- * Find the best backend by grouping the servers by priority, and then applying
- * selection criteria to the best group.
- *
- * @param backends: vector of RWBackend*
- * @param select:   selection function
- * @param master_accept_reads: NOTE: even if this is false, in some cases a master can
- *                             still be selected for reads.
- *
- * @return Valid iterator into argument backends, or end(backends) if empty
- */
-mxs::PRWBackends::iterator find_best_backend(mxs::PRWBackends& backends,
-                                             BackendSelectFunction select,
-                                             bool masters_accepts_reads);
-
 /*
  * The following are implemented in rwsplit_tmp_table_multi.c
  */
@@ -427,3 +412,11 @@ void close_all_connections(mxs::PRWBackends& backends);
  * @return String representation of the error
  */
 std::string extract_error(GWBUF* buffer);
+
+/**
+ * Check if replication lag is below acceptable levels
+ */
+static inline bool rpl_lag_is_ok(mxs::RWBackend* backend, int max_rlag)
+{
+    return max_rlag == SERVER::RLAG_UNDEFINED || backend->server()->rlag <= max_rlag;
+}

server/modules/routing/readwritesplit/rwsplit_route_stmt.cc

@@ -633,14 +633,6 @@ bool RWSplitSession::route_session_write(GWBUF* querybuf, uint8_t command, uint3
     return nsucc;
 }
 
-/**
- * Check if replication lag is below acceptable levels
- */
-static inline bool rpl_lag_is_ok(RWBackend* backend, int max_rlag)
-{
-    return max_rlag == SERVER::RLAG_UNDEFINED || backend->server()->rlag <= max_rlag;
-}
-
 RWBackend* RWSplitSession::get_hinted_backend(char* name)
 {
     RWBackend* rval = nullptr;
@@ -661,49 +653,6 @@ RWBackend* RWSplitSession::get_hinted_backend(char* name)
     return rval;
 }
 
-RWBackend* RWSplitSession::get_slave_backend(int max_rlag)
-{
-    // create a list of useable backends (includes masters, function name is a bit off),
-    // then feed that list to compare.
-    PRWBackends candidates;
-    auto counts = get_slave_counts(m_raw_backends, m_current_master);
-
-    for (auto& backend : m_raw_backends)
-    {
-        bool can_take_slave_into_use = !backend->in_use() && can_recover_servers()
-            && backend->can_connect() && counts.second < m_router->max_slave_count()
-            && (backend->is_slave() || backend->is_master());
-
-        bool master_or_slave = backend->is_master() || backend->is_slave();
-        bool is_usable = backend->in_use() || can_take_slave_into_use;
-        bool rlag_ok = rpl_lag_is_ok(backend, max_rlag);
-
-        if (master_or_slave && is_usable)
-        {
-            if (rlag_ok)
-            {
-                candidates.push_back(backend);
-                if (max_rlag > 0)
-                {
-                    // Replication lag discrimination is on and the server passed.
-                    backend->change_rlag_state(SERVER::RLagState::BELOW_LIMIT, max_rlag);
-                }
-            }
-            else
-            {
-                // The server is otherwise usable except it's lagging too much.
-                backend->change_rlag_state(SERVER::RLagState::ABOVE_LIMIT, max_rlag);
-            }
-        }
-    }
-
-    PRWBackends::const_iterator rval = find_best_backend(candidates,
-                                                         m_config.backend_select_fct,
-                                                         m_config.master_accept_reads);
-
-    return (rval == candidates.end()) ? nullptr : *rval;
-}
-
 RWBackend* RWSplitSession::get_master_backend()
 {
     RWBackend* rval = nullptr;

server/modules/routing/readwritesplit/rwsplit_select_backends.cc

@@ -228,47 +228,54 @@ int get_backend_priority(RWBackend* backend, bool masters_accepts_reads)
     return priority;
 }
 
-/**
+RWBackend* RWSplitSession::get_slave_backend(int max_rlag)
- * @brief Find the best slave candidate for routing reads.
- *
- * @param backends All backends
- * @param select   Server selection function
- * @param masters_accepts_reads
- *
- * @return iterator to the best slave or backends.end() if none found
- */
-PRWBackends::iterator find_best_backend(PRWBackends& backends,
-                                        BackendSelectFunction select,
-                                        bool masters_accepts_reads)
 {
-    // Group backends by priority and rank (lower is better). The set of best backends will then compete.
-    int best_priority {INT_MAX};
-    auto best_rank = std::numeric_limits<int64_t>::max();
     thread_local PRWBackends candidates;
     candidates.clear();
 
-    for (auto& psBackend : backends)
+    auto counts = get_slave_counts(m_raw_backends, m_current_master);
-    {
+    int best_priority {INT_MAX};
-        int priority = get_backend_priority(psBackend, masters_accepts_reads);
+    auto best_rank = std::numeric_limits<int64_t>::max();
-        auto rank = psBackend->server()->rank();
 
-        if (rank < best_rank || (rank == best_rank && priority < best_priority))
+    // Create a list of backends valid for read operations
+    for (auto& backend : m_raw_backends)
+    {
+        bool can_take_slave_into_use = !backend->in_use() && can_recover_servers()
+            && backend->can_connect() && counts.second < m_router->max_slave_count()
+            && (backend->is_slave() || backend->is_master());
+
+        bool master_or_slave = backend->is_master() || backend->is_slave();
+        bool is_usable = backend->in_use() || can_take_slave_into_use;
+        bool rlag_ok = rpl_lag_is_ok(backend, max_rlag);
+        int priority = get_backend_priority(backend, m_config.master_accept_reads);
+        auto rank = backend->server()->rank();
+
+        if (master_or_slave && is_usable && rlag_ok)
         {
-            candidates.clear();
+            if (rank < best_rank || (rank == best_rank && priority < best_priority))
-            best_rank = rank;
+            {
-            best_priority = priority;
+                candidates.clear();
+                best_rank = rank;
+                best_priority = priority;
+            }
+
+            if (rank == best_rank && priority == best_priority)
+            {
+                candidates.push_back(backend);
+            }
         }
 
-        if (rank == best_rank && priority == best_priority)
+        if (max_rlag > 0)
         {
-            candidates.push_back(psBackend);
+            auto state = rlag_ok ? SERVER::RLagState::ABOVE_LIMIT : SERVER::RLagState::BELOW_LIMIT;
+            backend->change_rlag_state(state, max_rlag);
         }
     }
 
-    auto best = select(candidates);
+    // Let the slave selection function pick the best server
-    auto rval = std::find(backends.begin(), backends.end(), *best);
+    PRWBackends::const_iterator rval = m_config.backend_select_fct(candidates);
 
-    return rval;
+    return (rval == candidates.end()) ? nullptr : *rval;
 }
 
 void add_backend_with_rank(RWBackend* backend, PRWBackends* candidates, int64_t* best_rank)