Merge commit 'a60bd376108f71fccf40001c1496f32c11137fe4' into develop

maxscale-system-test/CMakeLists.txt

@@ -982,6 +982,9 @@ add_test_executable(mxs2520_master_read_reconnect.cpp mxs2520_master_read_reconn
 # MXS-2464: Crash in route_stored_query with ReadWriteSplit
 add_test_executable(mxs2464_sescmd_reconnect.cpp mxs2464_sescmd_reconnect mxs2464_sescmd_reconnect LABELS REPL_BACKEND readwritesplit)
 
+# MXS-2563: Failing debug assertion at rwsplitsession.cc:1129 : m_expected_responses == 0
+add_test_executable(mxs2563_concurrent_slave_failure.cpp mxs2563_concurrent_slave_failure mxs2563_concurrent_slave_failure LABELS REPL_BACKEND readwritesplit)
+
 ############################################
 # BEGIN: binlogrouter and avrorouter tests #
 ############################################

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

@@ -0,0 +1,26 @@
+[maxscale]
+threads=###threads###
+log_info=1
+
+###server###
+
+[MariaDB-Monitor]
+type=monitor
+module=mariadbmon
+servers=###server_line###
+user=maxskysql
+password=skysql
+monitor_interval=1000
+
+[RW-Split-Router]
+type=service
+router=readwritesplit
+servers=###server_line###
+user=maxskysql
+password=skysql
+
+[RW-Split-Listener]
+type=listener
+service=RW-Split-Router
+protocol=MySQLClient
+port=4006

maxscale-system-test/mxs2563_concurrent_slave_failure.cpp

@@ -0,0 +1,35 @@
+/**
+ * MXS-2563: Failing debug assertion at rwsplitsession.cc:1129 : m_expected_responses == 0
+ * https://jira.mariadb.org/browse/MXS-2563
+ */
+
+#include "testconnections.h"
+
+int main(int argc, char* argv[])
+{
+    TestConnections test(argc, argv);
+
+    test.maxctrl("alter monitor MariaDB-Monitor monitor_interval 99999");
+
+    auto conn = test.maxscales->rwsplit();
+    conn.connect();
+    conn.query("SET @a = (SELECT SLEEP(1))");
+
+    std::thread thr(
+        [&test]() {
+            sleep(5);
+            test.repl->stop_node(2);
+            test.repl->stop_node(3);
+            sleep(5);
+            test.repl->start_node(2);
+            test.repl->start_node(3);
+        });
+    // Should go to server2
+    conn.query("SELECT SLEEP(20)");
+    thr.join();
+
+
+    test.maxctrl("alter monitor MariaDB-Monitor monitor_interval 1000");
+
+    return test.global_result;
+}

server/core/config.cc

@@ -3731,8 +3731,10 @@ void config_add_defaults(CONFIG_CONTEXT* ctx, const MXS_MODULE_PARAM* params)
         {
             if (params[i].default_value && !ctx->m_parameters.contains(params[i].name))
             {
-                bool rv = config_add_param(ctx, params[i].name, params[i].default_value);
+                std::string key = params[i].name;
-                MXS_ABORT_IF_FALSE(rv);
+                std::string value = params[i].default_value;
+                config_fix_param(params, key, &value);
+                ctx->m_parameters.set(key, value);
             }
         }
     }

server/modules/monitor/mariadbmon/cluster_manipulation.cc

@@ -1015,6 +1015,18 @@ void MariaDBMonitor::wait_cluster_stabilization(GeneralOpData& op, const ServerA
                            "connect to '%s' within the time limit.";
         MXS_WARNING(MSG, fails, new_master->name(), repl_fails.size(), query_fails.size(),
                     unconfirmed.size(), new_master->name());
+
+        // If any of the unconfirmed slaves have error messages in their slave status, print them. They
+        // may explain what went wrong.
+        for (auto failed_slave : unconfirmed)
+        {
+            auto slave_conn = failed_slave->slave_connection_status_host_port(new_master);
+            if (slave_conn && !slave_conn->last_error.empty())
+            {
+                MXB_WARNING("%s did not connect because of error: '%s'",
+                            slave_conn->to_short_string().c_str(), slave_conn->last_error.c_str());
+            }
+        }
     }
     time_remaining -= timer.lap();
 }

server/modules/routing/readwritesplit/rwsplitsession.cc

@@ -1151,34 +1151,23 @@ bool RWSplitSession::handle_error_new_connection(DCB* backend_dcb, GWBUF* errmsg
         mxb_assert(m_expected_responses > 0);
         m_expected_responses--;
 
-        /**
+        // Route stored queries if this was the last server we expected a response from
-         * A query was sent through the backend and it is waiting for a reply.
+        route_stored = m_expected_responses == 0;
-         * Try to reroute the statement to a working server or send an error
-         * to the client.
-         */
-        GWBUF* stored = m_current_query.release();
 
-        if (stored && m_config.retry_failed_reads)
+        if (!backend->has_session_commands())
         {
-            mxb_assert(m_expected_responses == 0);
+            // The backend was busy executing command and the client is expecting a response.
-            MXS_INFO("Re-routing failed read after server '%s' failed", backend->name());
+            if (m_current_query.get() && m_config.retry_failed_reads)
-            retry_query(stored, 0);
-        }
-        else
-        {
-            gwbuf_free(stored);
-
-            if (!backend->has_session_commands())
             {
-                /** The backend was not executing a session command so the client
+                MXS_INFO("Re-routing failed read after server '%s' failed", backend->name());
-                 * is expecting a response. Send an error so they know to proceed. */
+                route_stored = false;
-                m_client->func.write(m_client, gwbuf_clone(errmsg));
+                retry_query(m_current_query.release(), 0);
             }
-
+            else
-            if (m_expected_responses == 0)
             {
-                // This was the last response, try to route pending queries
+                // Send an error so that the client knows to proceed.
-                route_stored = true;
+                m_client->func.write(m_client, gwbuf_clone(errmsg));
+                m_current_query.reset();
             }
         }
     }