diff --git a/CMakeLists.txt b/CMakeLists.txt index b74e89f2d..50527978b 100644 --- a/CMakeLists.txt +++ b/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 -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() +# Always build Connector-C from a known good commit +include(cmake/BuildMariaDBConnector.cmake) +include_directories(BEFORE ${MARIADB_CONNECTOR_INCLUDE_DIR}) include(cmake/BuildJansson.cmake) include(cmake/BuildMicroHttpd.cmake) diff --git a/Documentation/Filters/Query-Log-All-Filter.md b/Documentation/Filters/Query-Log-All-Filter.md index 4b27e7516..ee6b80702 100644 --- a/Documentation/Filters/Query-Log-All-Filter.md +++ b/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 diff --git a/cmake/defaults.cmake b/cmake/defaults.cmake index 6002be775..2c21c4b6e 100644 --- a/cmake/defaults.cmake +++ b/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)") diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 6e6903b84..748ebe651 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/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) diff --git a/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp b/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp index 9ce63da30..d73ac7d04 100644 --- a/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp +++ b/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()); diff --git a/maxscale-system-test/local_address.cpp b/maxscale-system-test/local_address.cpp index bfe6f65d9..d9ddf22c6 100644 --- a/maxscale-system-test/local_address.cpp +++ b/maxscale-system-test/local_address.cpp @@ -73,8 +73,9 @@ string extract_ip(string s) void get_maxscale_ips(TestConnections& test, vector* 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& 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,24 +302,34 @@ void run_test(TestConnections& test, const string& ip1, const string& ip2) test.maxscales->disconnect(); test.stop_maxscale(); - test.tprintf("\n"); - test.tprintf("WARNING: Other IP-address not tested, as usable IP-address not available."); - + if (ips.size() > 1) + { #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.", - ip2.c_str()); + test.tprintf("\n"); + test.tprintf("\nTesting with local_address=%s, bob should be able to access, alice not.", + ip2.c_str()); - string local_address_ip2 = "local_address=" + ip2; - start_maxscale_with_local_address(test, local_address_ip1, local_address_ip2); - test.connect_maxscale(); + string local_address_ip2 = "local_address=" + ip2; + start_maxscale_with_local_address(test, local_address_ip1, local_address_ip2); + test.connect_maxscale(); - test_connecting(test, zUser1, zPassword1, ip1.c_str(), false); - test_connecting(test, zUser2, zPassword2, ip2.c_str(), true); + test_connecting(test, zUser1, zPassword1, ip1.c_str(), false); + test_connecting(test, zUser2, zPassword2, ip2.c_str(), true); - test.maxscales->disconnect(); - test.stop_maxscale(); + 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 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; diff --git a/maxscale-system-test/mxs1653_ps_hang.cpp b/maxscale-system-test/mxs1653_ps_hang.cpp new file mode 100644 index 000000000..6bfaa66b0 --- /dev/null +++ b/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; +} diff --git a/maxscale-system-test/mxs280_select_outfile.cpp b/maxscale-system-test/mxs280_select_outfile.cpp index 2ffe05359..2c4e27311 100644 --- a/maxscale-system-test/mxs280_select_outfile.cpp +++ b/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; diff --git a/server/core/load_utils.cc b/server/core/load_utils.cc index d21522e0c..1f34b49e1 100644 --- a/server/core/load_utils.cc +++ b/server/core/load_utils.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include #include diff --git a/server/core/modutil.cc b/server/core/modutil.cc index 7618edf40..568fa99cf 100644 --- a/server/core/modutil.cc +++ b/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; } diff --git a/server/modules/monitor/mariadbmon/mysql_mon.cc b/server/modules/monitor/mariadbmon/mysql_mon.cc index cd1a8f79a..0c83bf942 100644 --- a/server/modules/monitor/mariadbmon/mysql_mon.cc +++ b/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); diff --git a/server/modules/protocol/MySQL/CMakeLists.txt b/server/modules/protocol/MySQL/CMakeLists.txt index 31730dc9b..a9ceabada 100644 --- a/server/modules/protocol/MySQL/CMakeLists.txt +++ b/server/modules/protocol/MySQL/CMakeLists.txt @@ -5,4 +5,7 @@ install_module(mysqlcommon core) add_subdirectory(mariadbbackend) add_subdirectory(mariadbclient) -add_subdirectory(test) + +if (BUILD_TESTS) + add_subdirectory(test) +endif() diff --git a/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc b/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc index 31d9ea6ac..48463ba15 100644 --- a/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc +++ b/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,41 +95,59 @@ void process_sescmd_response(RWSplitSession* rses, SRWBackend& backend, backend->add_ps_handle(id, resp.id); } - if (rses->recv_sescmd < rses->sent_sescmd && - id == rses->recv_sescmd + 1 && - (!rses->current_master || !rses->current_master->in_use() || // Session doesn't have a master - rses->current_master == backend)) // This is the master's response + if (rses->recv_sescmd < rses->sent_sescmd && id == rses->recv_sescmd + 1) { - /** First reply to this session command, route it to the client */ - ++rses->recv_sescmd; - - /** 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 (!rses->current_master || !rses->current_master->in_use() || // Session doesn't have a master + rses->current_master == backend) // This is the master's response { - /** 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; + /** 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 (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; + } + + // 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)) + { + *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 + else if (discard_if_response_differs(backend, rses->sescmd_responses[id], cmd)) + { + *pReconnect = true; + } + + if (discard) { - /** 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) - { - 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; - } } } } diff --git a/server/modules/routing/readwritesplit/rwsplitsession.hh b/server/modules/routing/readwritesplit/rwsplitsession.hh index 3c41f67a2..cc595fc1e 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.hh +++ b/server/modules/routing/readwritesplit/rwsplitsession.hh @@ -97,6 +97,9 @@ typedef std::list SRWBackendList; typedef std::tr1::unordered_set TableSet; typedef std::map ResponseMap; +/** List of slave responses that arrived before the master */ +typedef std::list< std::pair > SlaveResponseList; + /** Map of COM_STMT_EXECUTE targets by internal ID */ typedef std::tr1::unordered_map 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*/