Merge branch '2.3' into 2.4

maxscale-system-test/CMakeLists.txt

@@ -961,6 +961,9 @@ add_test_executable(mxs2273_being_drained.cpp mxs2273_being_drained mxs2273_bein
 # MXS-2295: COM_CHANGE_USER does not clear out session command history
 add_test_executable(mxs2295_change_user_loop.cpp mxs2295_change_user_loop mxs2295_change_user_loop LABELS REPL_BACKEND readwritesplit)
 
+# Debug assertion due to double-closed when a slave's response differs from the master
+add_test_executable(crash_on_bad_sescmd.cpp crash_on_bad_sescmd crash_on_bad_sescmd LABELS readwritesplit REPL_BACKEND)
+
 # MXS-2300: Prune session command history
 add_test_executable(mxs2300_history_pruning.cpp mxs2300_history_pruning mxs2300_history_pruning LABELS readwritesplit REPL_BACKEND)
 

maxscale-system-test/cnf/maxscale.cnf.template.crash_on_bad_sescmd

@@ -0,0 +1,49 @@
+[maxscale]
+threads=auto
+
+[server1]
+type=server
+address=###node_server_IP_1###
+port=###node_server_port_1###
+protocol=MySQLBackend
+
+[server2]
+type=server
+address=###node_server_IP_2###
+port=###node_server_port_2###
+protocol=MySQLBackend
+
+[server3]
+type=server
+address=###node_server_IP_3###
+port=###node_server_port_3###
+protocol=MySQLBackend
+
+[server4]
+type=server
+address=###node_server_IP_4###
+port=###node_server_port_4###
+protocol=MySQLBackend
+
+[MySQL Monitor]
+type=monitor
+module=mysqlmon
+servers=server1,server2,server3,server4
+user=maxskysql
+password=skysql
+monitor_interval=1000
+
+[RW Split Router]
+type=service
+router=readwritesplit
+servers=server1,server2,server3,server4
+user=maxskysql
+password=skysql
+max_sescmd_history=20
+disable_sescmd_history=false
+
+[RW Split Listener]
+type=listener
+service=RW Split Router
+protocol=MySQLClient
+port=4006

maxscale-system-test/crash_on_bad_sescmd.cpp

@@ -0,0 +1,69 @@
+/**
+ * Double-close on bad session command result
+ */
+
+#include "testconnections.h"
+
+void run_test(TestConnections& test)
+{
+    Connection conn = test.maxscales->rwsplit();
+    conn.connect();
+
+    for (int i = 0; i <= 300 && test.global_result == 0; i++)
+    {
+        if (conn.query("SET @a = 1")
+            && conn.query("USE test")
+            && conn.query("SET SQL_MODE=''")
+            && conn.query("USE test")
+            && conn.query("SELECT @@last_insert_id")
+            && conn.query("SELECT 1")
+            && conn.query("USE test")
+            && conn.query("SELECT 1")
+            && conn.query("SET @a = 123")
+            && conn.query("BEGIN")
+            && conn.query("SELECT @a")
+            && conn.query("COMMIT")
+            && conn.query("SET @a = 321")
+            && conn.query("SELECT @a")
+            && conn.query("SET @a = 456")
+            && conn.query("START TRANSACTION READ ONLY")
+            && conn.query("SELECT @a")
+            && conn.query("COMMIT")
+            && conn.query("PREPARE ps FROM 'SELECT 1'")
+            && conn.query("EXECUTE ps")
+            && conn.query("DEALLOCATE PREPARE ps"))
+        {
+            conn.reset_connection();
+        }
+        else
+        {
+            break;
+        }
+    }
+}
+
+int main(int argc, char* argv[])
+{
+    TestConnections test(argc, argv);
+
+    std::vector<std::thread> threads;
+
+    for (int i = 0; i < 5; i++)
+    {
+        threads.emplace_back(run_test, std::ref(test));
+    }
+
+    for (int i = 0; i < 5; i++)
+    {
+        test.repl->stop_node(1 + i % 3);
+        test.repl->start_node(1 + i % 3);
+        sleep(1);
+    }
+
+    for (auto& a : threads)
+    {
+        a.join();
+    }
+
+    return test.global_result;
+}

server/modules/routing/readwritesplit/rwsplit_session_cmd.cc

@@ -67,7 +67,7 @@ static void discard_if_response_differs(RWBackend* backend,
                                         uint8_t slave_response,
                                         SSessionCommand sescmd)
 {
-    if (master_response != slave_response)
+    if (master_response != slave_response && backend->in_use())
     {
         uint8_t cmd = sescmd->get_command();
         std::string query = sescmd->to_string();
@@ -88,7 +88,6 @@ void RWSplitSession::process_sescmd_response(RWBackend* backend, GWBUF** ppPacke
 {
     if (backend->has_session_commands())
     {
-        mxb_assert(GWBUF_IS_COLLECTED_RESULT(*ppPacket));
         uint8_t cmd;
         gwbuf_copy_data(*ppPacket, MYSQL_HEADER_LEN, 1, &cmd);
         uint8_t command = backend->next_session_command()->get_command();
@@ -97,6 +96,8 @@ void RWSplitSession::process_sescmd_response(RWBackend* backend, GWBUF** ppPacke
         MXS_PS_RESPONSE resp = {};
         bool discard = true;
 
+        mxb_assert(GWBUF_IS_COLLECTED_RESULT(*ppPacket) || command == MXS_COM_CHANGE_USER);
+
         if (command == MXS_COM_STMT_PREPARE && cmd != MYSQL_REPLY_ERR)
         {
             // This should never fail or the backend protocol is broken