Clean up functions that use SRWBackend

Return values instead of objects. This removes the need to handle cases
where a reference to a "debug value" is returned.

Return SRWBackend values instead of passing output references. This
doubles as a false boolan return value when an empty reference is
returned.

server/modules/routing/readwritesplit/readwritesplit.cc

@@ -146,14 +146,6 @@ int rses_get_max_replication_lag(ROUTER_CLIENT_SES *rses)
     return conf_max_rlag;
 }
 
-namespace
-{
-
-/** This will never get used but it should catch faults in the code */
-static SRWBackend no_backend;
-
-}
-
 /**
  * @brief Find a back end reference that matches the given DCB
  *
@@ -165,7 +157,7 @@ static SRWBackend no_backend;
  *
  * @return backend reference pointer if succeed or NULL
  */
-SRWBackend& get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb)
+SRWBackend get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb)
 {
     ss_dassert(dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
     CHK_DCB(dcb);
@@ -184,7 +176,7 @@ SRWBackend& get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb)
 
     /** We should always have a valid backend reference */
     ss_dassert(false);
-    return no_backend;
+    return SRWBackend();
 }
 
 /**
@@ -335,7 +327,7 @@ static void handle_error_reply_client(MXS_SESSION *ses, ROUTER_CLIENT_SES *rses,
     mxs_session_state_t sesstate = ses->state;
     DCB *client_dcb = ses->client_dcb;
 
-    SRWBackend& bref = get_bref_from_dcb(rses, backend_dcb);
+    SRWBackend bref = get_bref_from_dcb(rses, backend_dcb);
 
     bref->close();
 
@@ -346,7 +338,7 @@ static void handle_error_reply_client(MXS_SESSION *ses, ROUTER_CLIENT_SES *rses,
     }
 }
 
-static bool reroute_stored_statement(ROUTER_CLIENT_SES *rses, SRWBackend& old, GWBUF *stored)
+static bool reroute_stored_statement(ROUTER_CLIENT_SES *rses, const SRWBackend& old, GWBUF *stored)
 {
     bool success = false;
 
@@ -420,7 +412,7 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst,
                                         DCB *backend_dcb, GWBUF *errmsg)
 {
     ROUTER_CLIENT_SES *myrses = *rses;
-    SRWBackend& bref = get_bref_from_dcb(myrses, backend_dcb);
+    SRWBackend bref = get_bref_from_dcb(myrses, backend_dcb);
 
     MXS_SESSION* ses = backend_dcb->session;
     CHK_SESSION(ses);
@@ -486,7 +478,8 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst,
     {
         succp = select_connect_backend_servers(myrses->rses_nbackends, max_nslaves,
                                                myrses->rses_config.slave_selection_criteria,
-                                               ses, inst, myrses, true);
+                                               ses, inst, myrses,
+                                               connection_type::SLAVE);
     }
 
     return succp;
@@ -596,7 +589,7 @@ bool route_stored_query(ROUTER_CLIENT_SES *rses)
  *
  * @return True if the complete response has been received
  */
-bool reply_is_complete(SRWBackend& bref, GWBUF *buffer)
+bool reply_is_complete(SRWBackend bref, GWBUF *buffer)
 {
     mysql_server_cmd_t cmd = mxs_mysql_current_command(bref->dcb()->session);
 
@@ -802,7 +795,8 @@ static MXS_ROUTER_SESSION *newSession(MXS_ROUTER *router_inst, MXS_SESSION *sess
 
     if (!select_connect_backend_servers(router_nservers, max_nslaves,
                                         client_rses->rses_config.slave_selection_criteria,
-                                        session, router, client_rses, false))
+                                        session, router, client_rses,
+                                        connection_type::ALL))
     {
         /**
          * Master and at least <min_nslaves> slaves must be found if the router is
@@ -1114,7 +1108,7 @@ static void clientReply(MXS_ROUTER *instance,
      *    and
      */
 
-    SRWBackend& bref = get_bref_from_dcb(router_cli_ses, backend_dcb);
+    SRWBackend bref = get_bref_from_dcb(router_cli_ses, backend_dcb);
 
     /** Statement was successfully executed, free the stored statement */
     session_clear_stmt(backend_dcb->session);
@@ -1154,7 +1148,7 @@ static void clientReply(MXS_ROUTER *instance,
                 router_cli_ses->client_dcb->session,
                 router_cli_ses->router,
                 router_cli_ses,
-                true);
+                connection_type::SLAVE);
         }
     }
 
@@ -1256,7 +1250,7 @@ static void handleError(MXS_ROUTER *instance,
     MXS_SESSION *session = problem_dcb->session;
     ss_dassert(session);
 
-    SRWBackend& bref = get_bref_from_dcb(rses, problem_dcb);
+    SRWBackend bref = get_bref_from_dcb(rses, problem_dcb);
 
     switch (action)
     {

server/modules/routing/readwritesplit/rwsplit_internal.hh

@@ -51,7 +51,7 @@ bool send_readonly_error(DCB *dcb);
  * The following are implemented in readwritesplit.c
  */
 int router_handle_state_switch(DCB *dcb, DCB_REASON reason, void *data);
-SRWBackend& get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb);
+SRWBackend get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb);
 int rses_get_max_slavecount(ROUTER_CLIENT_SES *rses);
 int rses_get_max_replication_lag(ROUTER_CLIENT_SES *rses);
 
@@ -61,18 +61,17 @@ int rses_get_max_replication_lag(ROUTER_CLIENT_SES *rses);
 
 bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
                        GWBUF *querybuf);
-bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
+SRWBackend get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
-                        char *name, int max_rlag, SRWBackend& target);
+                              char *name, int max_rlag);
 route_target_t get_route_target(ROUTER_CLIENT_SES *rses,
                                 uint32_t qtype, HINT *hint);
 void handle_multi_temp_and_load(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
                                 uint8_t packet_type, uint32_t *qtype);
-bool handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
+SRWBackend handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
-                          route_target_t route_target, SRWBackend& target);
+                                route_target_t route_target);
-bool handle_slave_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
+SRWBackend handle_slave_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses);
-                            SRWBackend& target);
 bool handle_master_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
-                             SRWBackend& target);
+                             SRWBackend* dest);
 bool handle_got_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
                        GWBUF *querybuf, SRWBackend& target, bool store);
 bool route_session_write(ROUTER_CLIENT_SES *rses, GWBUF *querybuf, uint8_t command);
@@ -82,13 +81,21 @@ void process_sescmd_response(ROUTER_CLIENT_SES* rses, SRWBackend& bref,
 /*
  * The following are implemented in rwsplit_select_backends.c
  */
+
+/** What sort of connections should be create */
+enum connection_type
+{
+    ALL,
+    SLAVE
+};
+
 bool select_connect_backend_servers(int router_nservers,
                                     int max_nslaves,
                                     select_criteria_t select_criteria,
                                     MXS_SESSION *session,
                                     ROUTER_INSTANCE *router,
                                     ROUTER_CLIENT_SES *rses,
-                                    bool active_session);
+                                    connection_type type);
 
 /*
  * The following are implemented in rwsplit_tmp_table_multi.c

server/modules/routing/readwritesplit/rwsplit_route_stmt.cc

@@ -175,16 +175,22 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
         if (TARGET_IS_NAMED_SERVER(route_target) ||
             TARGET_IS_RLAG_MAX(route_target))
         {
-            succp = handle_hinted_target(rses, querybuf, route_target, target);
+            if ((target = handle_hinted_target(rses, querybuf, route_target)))
+            {
+                succp = true;
+            }
         }
         else if (TARGET_IS_SLAVE(route_target))
         {
-            succp = handle_slave_is_target(inst, rses, target);
+            if ((target = handle_slave_is_target(inst, rses)))
-            store_stmt = rses->rses_config.retry_failed_reads;
+            {
+                succp = true;
+                store_stmt = rses->rses_config.retry_failed_reads;
+            }
         }
         else if (TARGET_IS_MASTER(route_target))
         {
-            succp = handle_master_is_target(inst, rses, target);
+            succp = handle_master_is_target(inst, rses, &target);
 
             if (!rses->rses_config.strict_multi_stmt &&
                 rses->target_node == rses->current_master)
@@ -324,8 +330,8 @@ bool route_session_write(ROUTER_CLIENT_SES *rses, GWBUF *querybuf, uint8_t comma
  *
  * @return True if a backend was found
  */
-bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
+SRWBackend get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
-                        char *name, int max_rlag, SRWBackend& target)
+                              char *name, int max_rlag)
 {
     CHK_CLIENT_RSES(rses);
 
@@ -334,8 +340,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
     {
         MXS_DEBUG("In READ ONLY transaction, using server '%s'",
                   rses->target_node->server()->unique_name);
-        target = rses->target_node;
+        return rses->target_node;
-        return true;
     }
 
     bool succp = false;
@@ -360,8 +365,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                  SERVER_IS_RELAY_SERVER(bref->server()) ||
                  SERVER_IS_MASTER(bref->server())))
             {
-                target = bref;
+                return bref;
-                return true;
             }
         }
 
@@ -369,10 +373,10 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
         btype = BE_SLAVE;
     }
 
+    SRWBackend rval;
+
     if (btype == BE_SLAVE)
     {
-        SRWBackend candidate_bref;
-
         for (SRWBackendList::iterator it = rses->backends.begin();
              it != rses->backends.end(); it++)
         {
@@ -391,7 +395,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
              * If there are no candidates yet accept both master or
              * slave.
              */
-            else if (!candidate_bref)
+            else if (!rval)
             {
                 /**
                  * Ensure that master has not changed during
@@ -400,8 +404,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                 if (SERVER_IS_MASTER(bref->server()) && bref == rses->current_master)
                 {
                     /** found master */
-                    candidate_bref = bref;
+                    rval = bref;
-                    succp = true;
                 }
                 /**
                  * Ensure that max replication lag is not set
@@ -413,15 +416,14 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                           bref->server()->rlag <= max_rlag))
                 {
                     /** found slave */
-                    candidate_bref = bref;
+                    rval = bref;
-                    succp = true;
                 }
             }
             /**
              * If candidate is master, any slave which doesn't break
              * replication lag limits replaces it.
              */
-            else if (SERVER_IS_MASTER(candidate_bref->server()) &&
+            else if (SERVER_IS_MASTER(rval->server()) &&
                      SERVER_IS_SLAVE(bref->server()) &&
                      (max_rlag == MAX_RLAG_UNDEFINED ||
                       (bref->server()->rlag != MAX_RLAG_NOT_AVAILABLE &&
@@ -429,8 +431,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                      !rses->rses_config.master_accept_reads)
             {
                 /** found slave */
-                candidate_bref = bref;
+                rval = bref;
-                succp = true;
             }
             /**
              * When candidate exists, compare it against the current
@@ -445,8 +446,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                     (bref->server()->rlag != MAX_RLAG_NOT_AVAILABLE &&
                      bref->server()->rlag <= max_rlag))
                 {
-                    candidate_bref = check_candidate_bref(candidate_bref, bref,
+                    rval = check_candidate_bref(rval, bref, rses->rses_config.slave_selection_criteria);
-                                                          rses->rses_config.slave_selection_criteria);
                 }
                 else
                 {
@@ -457,13 +457,6 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                 }
             }
         } /*<  for */
-
-        /** Assign selected DCB's pointer value */
-        if (candidate_bref)
-        {
-            target = candidate_bref;
-        }
-
     }
     /**
      * If target was originally master only then the execution jumps
@@ -483,8 +476,7 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
             {
                 if (SERVER_IS_MASTER(&server))
                 {
-                    target = master_bref;
+                    rval = master_bref;
-                    succp = true;
                 }
                 else
                 {
@@ -492,19 +484,17 @@ bool get_target_backend(ROUTER_CLIENT_SES *rses, backend_type_t btype,
                               "and can't be chosen as the master.",
                               master_bref->server()->unique_name,
                               STRSRVSTATUS(&server));
-                    succp = false;
                 }
             }
             else
             {
                 MXS_ERROR("Server '%s' is not in use and can't be chosen as the master.",
                           master_bref->server()->unique_name);
-                succp = false;
             }
         }
     }
 
-    return succp;
+    return rval;
 }
 
 /**
@@ -811,12 +801,11 @@ handle_multi_temp_and_load(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
  *
  *  @return bool - true if succeeded, false otherwise
  */
-bool handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
+SRWBackend handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
-                          route_target_t route_target, SRWBackend& target)
+                                route_target_t route_target)
 {
     char *named_server = NULL;
     int rlag_max = MAX_RLAG_UNDEFINED;
-    bool succp;
 
     HINT* hint = querybuf->hint;
 
@@ -859,9 +848,9 @@ bool handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
      * Search backend server by name or replication lag.
      * If it fails, then try to find valid slave or master.
      */
-    succp = get_target_backend(rses, btype, named_server, rlag_max, target);
+    SRWBackend target = get_target_backend(rses, btype, named_server, rlag_max);
 
-    if (!succp)
+    if (!target)
     {
         if (TARGET_IS_NAMED_SERVER(route_target))
         {
@@ -876,7 +865,8 @@ bool handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
                      "find such a slave.", rlag_max);
         }
     }
-    return succp;
+
+    return target;
 }
 
 /**
@@ -890,24 +880,21 @@ bool handle_hinted_target(ROUTER_CLIENT_SES *rses, GWBUF *querybuf,
  *
  *  @return bool - true if succeeded, false otherwise
  */
-bool handle_slave_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
+SRWBackend handle_slave_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses)
-                            SRWBackend& target)
 {
     int rlag_max = rses_get_max_replication_lag(rses);
+    SRWBackend target = get_target_backend(rses, BE_SLAVE, NULL, rlag_max);
 
-    /**
+    if (target)
-     * Search suitable backend server, get DCB in target_dcb
-     */
-    if (get_target_backend(rses, BE_SLAVE, NULL, rlag_max, target))
     {
         atomic_add_uint64(&inst->stats.n_slave, 1);
-        return true;
     }
     else
     {
         MXS_INFO("Was supposed to route to slave but finding suitable one failed.");
-        return false;
     }
+
+    return target;
 }
 
 /**
@@ -980,12 +967,12 @@ static void log_master_routing_failure(ROUTER_CLIENT_SES *rses, bool found,
  *  @return bool - true if succeeded, false otherwise
  */
 bool handle_master_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
-                             SRWBackend& target)
+                             SRWBackend* dest)
 {
-    DCB *curr_master_dcb = NULL;
+    SRWBackend target = get_target_backend(rses, BE_MASTER, NULL, MAX_RLAG_UNDEFINED);
-    bool succp = get_target_backend(rses, BE_MASTER, NULL, MAX_RLAG_UNDEFINED, target);
+    bool succp = true;
 
-    if (succp && target == rses->current_master)
+    if (target && target == rses->current_master)
     {
         atomic_add_uint64(&inst->stats.n_master, 1);
     }
@@ -1008,6 +995,7 @@ bool handle_master_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
         }
     }
 
+    *dest = target;
     return succp;
 }
 

server/modules/routing/readwritesplit/rwsplit_select_backends.cc

@@ -133,7 +133,7 @@ bool select_connect_backend_servers(int router_nservers,
                                     MXS_SESSION *session,
                                     ROUTER_INSTANCE *router,
                                     ROUTER_CLIENT_SES *rses,
-                                    bool active_session)
+                                    connection_type type)
 {
     /* get the root Master */
     SERVER_REF *master_backend = get_root_master(rses);
@@ -156,7 +156,7 @@ bool select_connect_backend_servers(int router_nservers,
      * Master is already connected or we don't have a master. The function was
      * called because new slaves must be selected to replace failed ones.
      */
-    bool master_connected = active_session || rses->current_master;
+    bool master_connected = type == SLAVE || rses->current_master;
 
     /** Check slave selection criteria and set compare function */
     int (*cmpfun)(const void *, const void *) = criteria_cmpfun[select_criteria];