From f7500cce9ef524b8a575ceb46f2d91fc08b10248 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Thu, 29 Jun 2017 09:22:22 +0300
Subject: [PATCH] Clean up route target resolution function

Cleaned up the function that resolves to which type of a server the query
should go.
---
 .../readwritesplit/rwsplit_route_stmt.cc      | 38 +++++++++----------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc
index ae6f089e5..a563660e0 100644
--- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc
+++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc
@@ -545,7 +545,7 @@ SRWBackend get_target_backend(RWSplitSession *rses, backend_type_t btype,
  *          if the query would otherwise be routed to slave.
  */
 route_target_t get_route_target(RWSplitSession *rses, uint8_t command,
-                                uint32_t qtype, HINT *hint)
+                                uint32_t qtype, HINT *query_hints)
 {
     bool trx_active = session_trx_is_active(rses->client_dcb->session);
     bool load_active = rses->load_data_state != LOAD_DATA_INACTIVE;
@@ -556,6 +556,9 @@ route_target_t get_route_target(RWSplitSession *rses, uint8_t command,
     {
         target = TARGET_MASTER;
     }
+    /**
+     * Prepared statements preparations should go to all servers
+     */
     else if (qc_query_is_type(qtype, QUERY_TYPE_PREPARE_STMT) ||
              qc_query_is_type(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) ||
              command == MYSQL_COM_STMT_CLOSE ||
@@ -564,7 +567,7 @@ route_target_t get_route_target(RWSplitSession *rses, uint8_t command,
         target = TARGET_ALL;
     }
     /**
-     * These queries are not affected by hints
+     * These queries should be routed to all servers
      */
     else if (!load_active &&
              (qc_query_is_type(qtype, QUERY_TYPE_SESSION_WRITE) ||
@@ -640,7 +643,7 @@ route_target_t get_route_target(RWSplitSession *rses, uint8_t command,
     }
     else if (session_trx_is_read_only(rses->client_dcb->session))
     {
-        /* Force TARGET_SLAVE for READ ONLY tranaction (active or ending) */
+        /* Force TARGET_SLAVE for READ ONLY transaction (active or ending) */
         target = TARGET_SLAVE;
     }
     else
@@ -673,14 +676,13 @@ route_target_t get_route_target(RWSplitSession *rses, uint8_t command,
         target = TARGET_MASTER;
     }
 
-    /** process routing hints */
-    while (hint != NULL)
+    /** Process routing hints */
+    for (HINT* hint = query_hints; hint; hint = hint->next)
     {
         if (hint->type == HINT_ROUTE_TO_MASTER)
         {
             target = TARGET_MASTER; /*< override */
-            MXS_DEBUG("%lu [get_route_target] Hint: route to master.",
-                      pthread_self());
+            MXS_DEBUG("Hint: route to master");
             break;
         }
         else if (hint->type == HINT_ROUTE_TO_NAMED_SERVER)
@@ -690,42 +692,38 @@ route_target_t get_route_target(RWSplitSession *rses, uint8_t command,
              * found, the oroginal target is chosen.
              */
             target |= TARGET_NAMED_SERVER;
-            MXS_DEBUG("%lu [get_route_target] Hint: route to "
-                      "named server : ",
-                      pthread_self());
+            MXS_DEBUG("Hint: route to named server: %s", (char*)hint->data);
         }
         else if (hint->type == HINT_ROUTE_TO_UPTODATE_SERVER)
         {
             /** not implemented */
+            ss_dassert(false);
         }
         else if (hint->type == HINT_ROUTE_TO_ALL)
         {
             /** not implemented */
+            ss_dassert(false);
         }
         else if (hint->type == HINT_PARAMETER)
         {
-            if (strncasecmp((char *)hint->data, "max_slave_replication_lag",
+            if (strncasecmp((char*)hint->data, "max_slave_replication_lag",
                             strlen("max_slave_replication_lag")) == 0)
             {
                 target |= TARGET_RLAG_MAX;
             }
             else
             {
-                MXS_ERROR("Unknown hint parameter "
-                          "'%s' when 'max_slave_replication_lag' "
-                          "was expected.",
-                          (char *)hint->data);
+                MXS_ERROR("Unknown hint parameter '%s' when "
+                          "'max_slave_replication_lag' was expected.",
+                          (char*)hint->data);
             }
         }
         else if (hint->type == HINT_ROUTE_TO_SLAVE)
         {
             target = TARGET_SLAVE;
-            MXS_DEBUG("%lu [get_route_target] Hint: route to "
-                      "slave.",
-                      pthread_self());
+            MXS_DEBUG("Hint: route to slave.");
         }
-        hint = hint->next;
-    } /*< while (hint != NULL) */
+    }
 
     return (route_target_t)target;
 }