Merge branch '2.2' into develop

CMakeLists.txt

@@ -33,7 +33,6 @@ include(cmake/CheckPlatform.cmake)
 check_dirs()
 find_package(OpenSSL)
 find_package(Valgrind)
-find_package(MariaDBConnector)
 find_package(Pandoc)
 find_package(TCMalloc)
 find_package(Jemalloc)
@@ -51,16 +50,9 @@ include(cmake/BuildPCRE2.cmake)
 
 include_directories(BEFORE ${PCRE2_INCLUDE_DIRS})
 
-# If the connector was not found, download and build it from source
+# Always build Connector-C from a known good commit
-if(NOT MARIADB_CONNECTOR_FOUND)
-  message(STATUS "Building MariaDB Connector-C from source.")
 include(cmake/BuildMariaDBConnector.cmake)
 include_directories(BEFORE ${MARIADB_CONNECTOR_INCLUDE_DIR})
-else()
-  # This is required as the core depends on the `connector-c` target
-  add_custom_target(connector-c)
-  message(STATUS "Using system Connector-C")
-endif()
 
 include(cmake/BuildJansson.cmake)
 include(cmake/BuildMicroHttpd.cmake)

Documentation/Filters/Query-Log-All-Filter.md

@@ -16,7 +16,7 @@ module=qlafilter
 
 [MyService]
 type=service
-router=readconnrouter
+router=readconnroute
 servers=server1
 user=myuser
 passwd=mypasswd

cmake/defaults.cmake

@@ -40,7 +40,7 @@ set(GCOV FALSE CACHE BOOL "Use gcov build flags")
 set(WITH_SCRIPTS TRUE CACHE BOOL "Install init.d scripts and ldconf configuration files")
 
 # Build tests
-set(BUILD_TESTS FALSE CACHE BOOL "Build tests")
+set(BUILD_TESTS TRUE CACHE BOOL "Build tests")
 
 # Build packages
 set(PACKAGE FALSE CACHE BOOL "Enable package building (this disables local installation of system files)")

maxscale-system-test/CMakeLists.txt

@@ -601,6 +601,10 @@ add_test_executable(mxs1585.cpp mxs1585 mxs1585 LABELS REPL_BACKEND)
  # https://jira.mariadb.org/browse/MXS-1643
 add_test_executable(mxs1643_extra_events.cpp mxs1643_extra_events mxs1643_extra_events LABELS REPL_BACKEND)
 
+# MXS-1653: sysbench failed to initialize w/ MaxScale read/write splitter
+# https://jira.mariadb.org/browse/MXS-1653
+add_test_executable(mxs1653_ps_hang.cpp mxs1653_ps_hang replication LABELS REPL_BACKEND)
+
 # 'namedserverfilter' test
 add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND)
 

maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp

@@ -198,7 +198,7 @@ std::string type_to_table_name(const char* type)
 
 static std::string unquote(std::string str)
 {
-    if (str[0] == '\"')
+    if (str[0] == '\"' ||str[0] == '\'')
     {
         str = str.substr(1, str.length() - 2);
     }
@@ -246,7 +246,11 @@ bool run_test(TestConnections& test)
                         std::string input = unquote(test_set[x].values[j]);
                         std::string output = row->value(field_name);
 
-                        if (input != output && (input != "NULL" || output != ""))
+                        if (input == output || (input == "NULL" && (output == "" || output == "0")))
+                        {
+                            // Expected result
+                        }
+                        else
                         {
                             test.tprintf("Result mismatch: %s(%s) => %s",
                                          test_set[x].types[i], input.c_str(), output.c_str());

maxscale-system-test/local_address.cpp

@@ -73,8 +73,9 @@ string extract_ip(string s)
 
 void get_maxscale_ips(TestConnections& test, vector<string>* pIps)
 {
+    static const char COMMAND[] = "export PATH=$PATH:/sbin:/usr/sbin; ip addr|fgrep inet|fgrep -v ::";
     int exit_code;
-    string output(test.maxscales->ssh_node_output(0, "ip addr|fgrep inet|fgrep -v ::", false, &exit_code));
+    string output(test.maxscales->ssh_node_output(0, COMMAND, false, &exit_code));
 
     to_collection(output, "\n", pIps);
     transform(pIps->begin(), pIps->end(), pIps->begin(), extract_ip);
@@ -258,10 +259,14 @@ void test_connecting(TestConnections& test,
     }
 }
 
-void run_test(TestConnections& test, const string& ip1, const string& ip2)
+void run_test(TestConnections& test, const vector<string>& ips)
 {
     test.maxscales->connect();
 
+    string ip1 = ips[0];
+    // If we do not have a proper second IP-address, we'll use an arbitrary one.
+    string ip2 = (ips.size() > 1) ? ips[1] : string("42.42.42.42");
+
     string local_ip = get_local_ip(test);
 
     const char* zUser1 = "alice";
@@ -297,9 +302,8 @@ void run_test(TestConnections& test, const string& ip1, const string& ip2)
     test.maxscales->disconnect();
     test.stop_maxscale();
 
-    test.tprintf("\n");
+    if (ips.size() > 1)
-    test.tprintf("WARNING: Other IP-address not tested, as usable IP-address not available.");
+    {
-
 #ifdef USABLE_SECOND_IP_ADDRESS_ON_MAXSCALE_NODE_IS_AVAILABLE
         test.tprintf("\n");
         test.tprintf("\nTesting with local_address=%s, bob should be able to access, alice not.",
@@ -314,8 +318,19 @@ void run_test(TestConnections& test, const string& ip1, const string& ip2)
 
         test.maxscales->disconnect();
         test.stop_maxscale();
+#else
+        test.tprintf("\n");
+        test.tprintf("WARNING: Other IP-address (%s) not tested, as IP-address currently "
+                     "not usable on VM.", ip2.c_str());
 #endif
     }
+    else
+    {
+        test.tprintf("\n");
+        test.tprintf("WARNING: Only one IP-address found on MaxScale node, 'local_address' "
+                     "not properly tested.");
+    }
+}
 
 }
 
@@ -326,13 +341,13 @@ int main(int argc, char** argv)
     vector<string> ips;
     get_maxscale_ips(test, &ips);
 
-    if (ips.size() >= 2)
+    if (ips.size() >= 1)
     {
-        run_test(test, ips[0], ips[1]);
+        run_test(test, ips);
     }
     else
     {
-        test.assert(false, "MaxScale node does not have at least two IP-addresses.");
+        test.assert(false, "MaxScale node does not have at least one IP-address.");
     }
 
     return test.global_result;

maxscale-system-test/mxs1653_ps_hang.cpp

@@ -0,0 +1,29 @@
+#include "testconnections.h"
+
+int main(int argc, char** argv)
+{
+    TestConnections test(argc, argv);
+
+    test.set_timeout(20);
+    test.maxscales->connect();
+
+    MYSQL_STMT* stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]);
+    std::string query = "COMMIT";
+    mysql_stmt_prepare(stmt, query.c_str(), query.size());
+    mysql_stmt_execute(stmt);
+    mysql_stmt_close(stmt);
+
+    stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]);
+    query = "BEGIN";
+    mysql_stmt_prepare(stmt, query.c_str(), query.size());
+    mysql_stmt_execute(stmt);
+    mysql_stmt_close(stmt);
+
+    test.set_timeout(30);
+    execute_query_silent(test.maxscales->conn_rwsplit[0], "PREPARE test FROM 'BEGIN'");
+    execute_query_silent(test.maxscales->conn_rwsplit[0], "EXECUTE test");
+
+    test.maxscales->disconnect();
+
+    return test.global_result;
+}

maxscale-system-test/mxs280_select_outfile.cpp

@@ -47,8 +47,6 @@ int main(int argc, char *argv[])
 
     Test->set_timeout(30);
     sleep(5);
-    Test->check_log_err(0, (char *) "Failed to execute session command in", true);
-    Test->check_log_err(0, (char *) "File '/tmp/t1.csv' already exists", true);
 
     int rval = Test->global_result;
     delete Test;

server/core/load_utils.cc

@@ -24,6 +24,7 @@
 #include <string.h>
 #include <dlfcn.h>
 #include <algorithm>
+#include <string>
 
 #include <maxscale/modinfo.h>
 #include <maxscale/log_manager.h>

server/core/modutil.cc

@@ -637,9 +637,11 @@ int modutil_count_signal_packets(GWBUF *reply, int n_found, bool* more_out, modu
     bool skip_next = state ? state->state : false;
     bool more = false;
     bool only_ok = true;
+    uint64_t num_packets = 0;
 
     while (offset < len)
     {
+        num_packets++;
         uint8_t header[MYSQL_HEADER_LEN + 5]; // Maximum size of an EOF packet
 
         gwbuf_copy_data(reply, offset, MYSQL_HEADER_LEN + 1, header);
@@ -720,7 +722,9 @@ int modutil_count_signal_packets(GWBUF *reply, int n_found, bool* more_out, modu
 
     *more_out = more;
 
-    if (only_ok && !more)
+    // Treat complete multi-statement result sets that consist of only OK packets as a single result set
+    // TODO: Review this, it doesn't look very convincing.
+    if (only_ok && !more && num_packets > 1)
     {
         total = 2;
     }

server/modules/monitor/mariadbmon/mysql_mon.cc

@@ -2372,7 +2372,7 @@ monitorMain(void *arg)
             }
         }
 
-        ss_dassert(handle->master == root_master);
+        ss_dassert(root_master == NULL || handle->master == root_master);
         ss_dassert(!root_master ||
                    ((root_master->server->status & (SERVER_SLAVE | SERVER_MASTER))
                     != (SERVER_SLAVE | SERVER_MASTER)));
@@ -2847,7 +2847,11 @@ static MXS_MONITORED_SERVER *get_replication_tree(MXS_MONITOR *mon, int num_serv
         current = ptr->server;
 
         node_id = current->master_id;
-        if (node_id < 1)
+
+        /** Either this node doesn't replicate from a master or the master
+         * where it replicates from is not configured to this monitor. */
+        if (node_id < 1 ||
+            getServerByNodeId(mon->monitored_servers, node_id) == NULL)
         {
             MXS_MONITORED_SERVER *find_slave;
             find_slave = getSlaveOfNodeId(mon->monitored_servers, current->node_id, ACCEPT_DOWN);

server/modules/protocol/MySQL/CMakeLists.txt

@@ -5,4 +5,7 @@ install_module(mysqlcommon core)
 
 add_subdirectory(mariadbbackend)
 add_subdirectory(mariadbclient)
+
+if (BUILD_TESTS)
   add_subdirectory(test)
+endif()

server/modules/routing/readwritesplit/rwsplit_session_cmd.cc

@@ -26,6 +26,52 @@
  * Functions for session command handling
  */
 
+
+static std::string extract_error(GWBUF* buffer)
+{
+    std::string rval;
+
+    if (MYSQL_IS_ERROR_PACKET(((uint8_t *)GWBUF_DATA(buffer))))
+    {
+        size_t replylen = MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(buffer));
+        char replybuf[replylen];
+        gwbuf_copy_data(buffer, 0, gwbuf_length(buffer), (uint8_t*)replybuf);
+        std::string err;
+        std::string msg;
+        err.append(replybuf + 8, 5);
+        msg.append(replybuf + 13, replylen - 4 - 5);
+        rval = err + ": " + msg;
+    }
+
+    return rval;
+}
+
+/**
+ * Discards the slave connection if its response differs from the master's response
+ *
+ * @param backend    The slave Backend
+ * @param master_cmd Master's reply
+ * @param slave_cmd  Slave's reply
+ *
+ * @return True if the responses were different and connection was discarded
+ */
+static bool discard_if_response_differs(SRWBackend backend, uint8_t master_cmd, uint8_t slave_cmd)
+{
+    bool rval = false;
+
+    if (master_cmd != slave_cmd)
+    {
+        MXS_WARNING("Slave server '%s': response (0x%02hhx) differs "
+                    "from master's response(0x%02hhx). Closing slave "
+                    "connection due to inconsistent session state.",
+                    backend->name(), slave_cmd, master_cmd);
+        backend->close(mxs::Backend::CLOSE_FATAL);
+        rval = true;
+    }
+
+    return rval;
+}
+
 void process_sescmd_response(RWSplitSession* rses, SRWBackend& backend,
                              GWBUF** ppPacket, bool* pReconnect)
 {
@@ -39,6 +85,7 @@ void process_sescmd_response(RWSplitSession* rses, SRWBackend& backend,
             uint8_t command = backend->next_session_command()->get_command();
             uint64_t id = backend->complete_session_command();
             MXS_PS_RESPONSE resp = {};
+            bool discard = true;
 
             if (command == MXS_COM_STMT_PREPARE && cmd != MYSQL_REPLY_ERR)
             {
@@ -48,42 +95,60 @@ void process_sescmd_response(RWSplitSession* rses, SRWBackend& backend,
                 backend->add_ps_handle(id, resp.id);
             }
 
-            if (rses->recv_sescmd < rses->sent_sescmd &&
+            if (rses->recv_sescmd < rses->sent_sescmd && id == rses->recv_sescmd + 1)
-                id == rses->recv_sescmd + 1 &&
+            {
-                (!rses->current_master || !rses->current_master->in_use() || // Session doesn't have a master
+                if (!rses->current_master || !rses->current_master->in_use() || // Session doesn't have a master
-                 rses->current_master == backend)) // This is the master's response
+                    rses->current_master == backend) // This is the master's response
                 {
                     /** First reply to this session command, route it to the client */
                     ++rses->recv_sescmd;
+                    discard = false;
 
                     /** Store the master's response so that the slave responses can
                      * be compared to it */
                     rses->sescmd_responses[id] = cmd;
 
-                if (command == MXS_COM_STMT_PREPARE)
+                    if (cmd == MYSQL_REPLY_ERR)
+                    {
+                        MXS_INFO("Session command no. %lu failed: %s",
+                                 id, extract_error(*ppPacket).c_str());
+                    }
+                    else if (command == MXS_COM_STMT_PREPARE)
                     {
                         /** Map the returned response to the internal ID */
                         MXS_INFO("PS ID %u maps to internal ID %lu", resp.id, id);
                         rses->ps_handles[resp.id] = id;
                     }
-            }
-            else
-            {
-                /** The reply to this session command has already been sent to
-                 * the client, discard it */
-                gwbuf_free(*ppPacket);
-                *ppPacket = NULL;
 
-                if (rses->sescmd_responses[id] != cmd)
+                    // Discard any slave connections that did not return the same result
+                    for (SlaveResponseList::iterator it = rses->slave_responses.begin();
+                         it != rses->slave_responses.end(); it++)
+                    {
+                        if (discard_if_response_differs(it->first, cmd, it->second))
                         {
-                    MXS_WARNING("Slave server '%s': response (0x%02hhx) differs "
-                                "from master's response(0x%02hhx). Closing slave "
-                                "connection due to inconsistent session state.",
-                                backend->name(), cmd, rses->sescmd_responses[id]);
-                    backend->close(mxs::Backend::CLOSE_FATAL);
                             *pReconnect = true;
                         }
                     }
+
+                    rses->slave_responses.clear();
+                }
+                else
+                {
+                    /** Record slave command so that the response can be validated
+                     * against the master's response when it arrives. */
+                    rses->slave_responses.push_back(std::make_pair(backend, cmd));
+                }
+            }
+            else if (discard_if_response_differs(backend, rses->sescmd_responses[id], cmd))
+            {
+                *pReconnect = true;
+            }
+
+            if (discard)
+            {
+                gwbuf_free(*ppPacket);
+                *ppPacket = NULL;
+            }
         }
     }
 }

server/modules/routing/readwritesplit/rwsplitsession.hh

@@ -97,6 +97,9 @@ typedef std::list<SRWBackend> SRWBackendList;
 typedef std::tr1::unordered_set<std::string> TableSet;
 typedef std::map<uint64_t, uint8_t>          ResponseMap;
 
+/** List of slave responses that arrived before the master */
+typedef std::list< std::pair<SRWBackend, uint8_t> > SlaveResponseList;
+
 /** Map of COM_STMT_EXECUTE targets by internal ID */
 typedef std::tr1::unordered_map<uint32_t, SRWBackend> ExecMap;
 
@@ -141,6 +144,7 @@ public:
     TableSet                temp_tables; /**< Set of temporary tables */
     mxs::SessionCommandList sescmd_list; /**< List of executed session commands */
     ResponseMap             sescmd_responses; /**< Response to each session command */
+    SlaveResponseList       slave_responses; /**< Slaves that replied before the master */
     uint64_t                sent_sescmd; /**< ID of the last sent session command*/
     uint64_t                recv_sescmd; /**< ID of the most recently completed session command */
     PSManager               ps_manager;  /**< Prepared statement manager*/