From 7840c86b7f14707ad6e69f79160e84bb20541ef8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Thu, 5 Oct 2017 16:03:00 +0300
Subject: [PATCH] Remove unnecessary result processing in readwritesplit

The result processing code did unnecessary work to confirm that the result
buffers are contiguous. The code also assumed that multiple packets can be
routed at the same time when in fact only one contiguous result packet is
returned at a time.

By assuming that the buffers are contiguous and contain only one packet,
most of the copying and buffer manipulation can be avoided.
---
 .../routing/readwritesplit/readwritesplit.cc  | 45 ++++++++++++++++---
 .../routing/readwritesplit/rwsplitsession.cc  | 10 ++---
 .../routing/readwritesplit/rwsplitsession.hh  | 10 ++---
 3 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/server/modules/routing/readwritesplit/readwritesplit.cc b/server/modules/routing/readwritesplit/readwritesplit.cc
index 067e49572..f9e0f8285 100644
--- a/server/modules/routing/readwritesplit/readwritesplit.cc
+++ b/server/modules/routing/readwritesplit/readwritesplit.cc
@@ -508,6 +508,25 @@ static bool route_stored_query(RWSplitSession *rses)
     return rval;
 }
 
+static bool is_eof(GWBUF* buffer)
+{
+    uint8_t* data = GWBUF_DATA(buffer);
+    return data[MYSQL_HEADER_LEN] == MYSQL_REPLY_EOF &&
+        gw_mysql_get_byte3(data) + MYSQL_HEADER_LEN == MYSQL_EOF_PACKET_LEN;
+}
+
+static bool is_large(GWBUF* buffer)
+{
+    return gw_mysql_get_byte3(GWBUF_DATA(buffer)) == GW_MYSQL_MAX_PACKET_LEN;
+}
+
+static bool more_results_exist(GWBUF* buffer)
+{
+    ss_dassert(is_eof(buffer));
+    uint16_t status = gw_mysql_get_byte2(GWBUF_DATA(buffer) + MYSQL_HEADER_LEN + 1 + 2);
+    return status & SERVER_MORE_RESULTS_EXIST;
+}
+
 /**
  * @brief Check if we have received a complete reply from the backend
  *
@@ -531,11 +550,23 @@ bool reply_is_complete(SRWBackend backend, GWBUF *buffer)
     }
     else
     {
-        bool more = false;
-        modutil_state state = backend->get_modutil_state();
-        int old_eof = backend->get_reply_state() == REPLY_STATE_RSET_ROWS ? 1 : 0;
-        int n_eof = modutil_count_signal_packets(buffer, old_eof, &more, &state);
-        backend->set_modutil_state(state);
+        bool large = backend->is_large_packet();
+        int n_eof = backend->get_reply_state() == REPLY_STATE_RSET_ROWS ? 1 : 0;
+
+        if (is_large(buffer))
+        {
+            large = true;
+        }
+        else if (large)
+        {
+            large = false;
+        }
+        else if (is_eof(buffer))
+        {
+            n_eof++;
+        }
+
+        backend->set_large_packet(large);
 
         if (n_eof == 0)
         {
@@ -557,7 +588,7 @@ bool reply_is_complete(SRWBackend backend, GWBUF *buffer)
             LOG_RS(backend, REPLY_STATE_DONE);
             backend->set_reply_state(REPLY_STATE_DONE);
 
-            if (more)
+            if (more_results_exist(buffer))
             {
                 /** The server will send more resultsets */
                 LOG_RS(backend, REPLY_STATE_START);
@@ -1116,6 +1147,8 @@ static void clientReply(MXS_ROUTER *instance,
                         GWBUF *writebuf,
                         DCB *backend_dcb)
 {
+    ss_dassert(GWBUF_IS_CONTIGUOUS(writebuf) &&
+        MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(writebuf)) + MYSQL_HEADER_LEN == gwbuf_length(writebuf));
     RWSplitSession *rses = (RWSplitSession *)router_session;
     DCB *client_dcb = backend_dcb->session->client_dcb;
 
diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc
index 737f698f9..21bc960ac 100644
--- a/server/modules/routing/readwritesplit/rwsplitsession.cc
+++ b/server/modules/routing/readwritesplit/rwsplitsession.cc
@@ -17,7 +17,7 @@
 RWBackend::RWBackend(SERVER_REF* ref):
     mxs::Backend(ref),
     m_reply_state(REPLY_STATE_DONE),
-    m_modutil_state(MODUTIL_STATE_INIT)
+    m_large_packet(false)
 {
 }
 
@@ -35,14 +35,14 @@ void RWBackend::set_reply_state(reply_state_t state)
     m_reply_state = state;
 }
 
-void RWBackend::set_modutil_state(const modutil_state& state)
+void RWBackend::set_large_packet(bool value)
 {
-    m_modutil_state = state;
+    m_large_packet = value;
 }
 
-modutil_state RWBackend::get_modutil_state() const
+bool RWBackend::is_large_packet() const
 {
-    return m_modutil_state;
+    return m_large_packet;
 }
 
 bool RWBackend::execute_session_command()
diff --git a/server/modules/routing/readwritesplit/rwsplitsession.hh b/server/modules/routing/readwritesplit/rwsplitsession.hh
index b1e3c741c..d2561c107 100644
--- a/server/modules/routing/readwritesplit/rwsplitsession.hh
+++ b/server/modules/routing/readwritesplit/rwsplitsession.hh
@@ -52,15 +52,15 @@ public:
     bool execute_session_command();
     bool write(GWBUF* buffer, response_type type = EXPECT_RESPONSE);
 
-    void set_modutil_state(const modutil_state& state);
-    modutil_state get_modutil_state() const;
+    bool is_large_packet() const;
+    void set_large_packet(bool value);
 
 private:
     reply_state_t    m_reply_state;
     BackendHandleMap m_ps_handles; /**< Internal ID to backend PS handle mapping */
-    modutil_state    m_modutil_state; /**< Used to store the state of the EOF packet
-                                       * calculation for result sets when the result
-                                       * contains very large rows */
+    bool             m_large_packet; /**< Used to store the state of the EOF packet
+                                      *calculation for result sets when the result
+                                      * contains very large rows */
 };
 
 typedef std::tr1::shared_ptr<RWBackend> SRWBackend;