From f3c84d84c780dfdcfa1ef4e059802179f1fc2b34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Sun, 8 Jul 2018 00:54:35 +0300
Subject: [PATCH] Fix transaction migration

The transaction migration in the case of a changed master never worked as
transaction replay would only be triggered when the master fails. To cover
this case, the transaction replay just needs to be started when the need
for a transaction migration is detected.

To help diagnose the behavior, the Trx class no longer logs a message when
a transaction is closed. This is now done by readwritesplit which has more
knowledge of the context in which the transaction is closed.
---
 .../readwritesplit/rwsplit_route_stmt.cc      | 33 +++++++++++++++++++
 .../routing/readwritesplit/rwsplitsession.cc  |  1 +
 .../routing/readwritesplit/rwsplitsession.hh  |  1 +
 server/modules/routing/readwritesplit/trx.hh  |  1 -
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc
index 709ac254c..9bf706958 100644
--- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc
+++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc
@@ -298,6 +298,26 @@ bool RWSplitSession::route_single_stmt(GWBUF *querybuf)
         else if (TARGET_IS_MASTER(route_target))
         {
             succp = handle_master_is_target(&target);
+
+            if (!succp && should_migrate_trx(target))
+            {
+                MXS_INFO("Starting transaction migration from '%s' to '%s'",
+                         m_current_master->name(), target->name());
+
+                /**
+                 * Stash the current query so that the transaction replay treats
+                 * it as if the query was interrupted.
+                 */
+                m_current_query.copy_from(querybuf);
+
+                /**
+                 * After the transaction replay has been started, the rest of
+                 * the query processing needs to be skipped. This is done to avoid
+                 * the error logging done when no valid target is found for a query
+                 * as well as to prevent retrying of queries in the wrong order.
+                 */
+                return start_trx_replay();
+            }
         }
 
         if (succp && target)
@@ -895,6 +915,19 @@ void RWSplitSession::replace_master(SRWBackend& target)
     m_qc.master_replaced();
 }
 
+bool RWSplitSession::should_migrate_trx(SRWBackend& target)
+{
+    return m_config->transaction_replay &&
+           // We have a target server and it's not the current master
+           target && target != m_current_master &&
+           // Transaction replay is not active (replay is only attempted once)
+           !m_is_replay_active &&
+           // We have an open transaction
+           session_trx_is_active(m_client->session) &&
+           // The transaction can be replayed
+           m_can_replay_trx;
+}
+
 /**
  * @brief Handle master is the target
  *
diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc
index 9ba318f49..521774c51 100644
--- a/server/modules/routing/readwritesplit/rwsplitsession.cc
+++ b/server/modules/routing/readwritesplit/rwsplitsession.cc
@@ -601,6 +601,7 @@ void RWSplitSession::clientReply(GWBUF *writebuf, DCB *backend_dcb)
     }
     else if (m_config->transaction_replay && session_trx_is_ending(m_client->session))
     {
+        MXS_INFO("Transaction complete");
         m_trx.close();
         m_can_replay_trx = true;
     }
diff --git a/server/modules/routing/readwritesplit/rwsplitsession.hh b/server/modules/routing/readwritesplit/rwsplitsession.hh
index 9ea1f8ff9..bbc1b2d82 100644
--- a/server/modules/routing/readwritesplit/rwsplitsession.hh
+++ b/server/modules/routing/readwritesplit/rwsplitsession.hh
@@ -188,6 +188,7 @@ private:
 
     bool should_replace_master(mxs::SRWBackend& target);
     void replace_master(mxs::SRWBackend& target);
+    bool should_migrate_trx(mxs::SRWBackend& target);
     void log_master_routing_failure(bool found, mxs::SRWBackend& old_master,
                                     mxs::SRWBackend& curr_master);
 
diff --git a/server/modules/routing/readwritesplit/trx.hh b/server/modules/routing/readwritesplit/trx.hh
index 8803f6121..8cea12100 100644
--- a/server/modules/routing/readwritesplit/trx.hh
+++ b/server/modules/routing/readwritesplit/trx.hh
@@ -131,7 +131,6 @@ public:
      */
     void close()
     {
-        MXS_INFO("Transaction is complete");
         m_checksum.reset();
         m_log.clear();
         m_size = 0;