From 8a526bf84fdd7e1b5bf161feff5a10d1fd12ffe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 20 Jun 2018 09:35:28 +0300 Subject: [PATCH 1/5] MXS-1926: Add test case Added a test case that reproduces the problem and verifies that it is solved. --- maxscale-system-test/CMakeLists.txt | 4 + ...axscale.cnf.template.mxs1926_killed_server | 57 ++++++++++ .../mxs1926_killed_server.cpp | 106 ++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100755 maxscale-system-test/cnf/maxscale.cnf.template.mxs1926_killed_server create mode 100644 maxscale-system-test/mxs1926_killed_server.cpp diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 0e91a1242..a54243e61 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -941,6 +941,10 @@ add_test_executable(mxs1836_show_eventTimes.cpp mxs1836_show_eventTimes mxs1836_ # https://jira.mariadb.org/browse/MXS-1889 add_test_executable(mxs1889.cpp mxs1889 mxs1889 LABELS REPL_BACKEND) +# MXS-1926: LOAD DATA LOCAL INFILE interrupted by server shutdown +# https://jira.mariadb.org/browse/MXS-1926 +add_test_executable(mxs1926_killed_server.cpp mxs1926_killed_server mxs1926_killed_server LABELS readwritesplit REPL_BACKEND) + # MXS-1932: Hidden files are not ignored # https://jira.mariadb.org/browse/MXS-1932 add_test_executable(mxs1932_hidden_cnf.cpp mxs1932_hidden_cnf replication LABELS REPL_BACKEND) diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mxs1926_killed_server b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1926_killed_server new file mode 100755 index 000000000..758a611eb --- /dev/null +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1926_killed_server @@ -0,0 +1,57 @@ +[maxscale] +threads=###threads### + +[MySQL Monitor] +type=monitor +module=mysqlmon +servers=server1,server2,server3,server4 +user=maxskysql +passwd=skysql +monitor_interval=1000 + +[RW Split Router] +type=service +router=readwritesplit +servers=server1,server2,server3,server4 +user=maxskysql +passwd=skysql + +[RW Split Listener] +type=listener +service=RW Split Router +protocol=MySQLClient +port=4006 + +[CLI] +type=service +router=cli + +[CLI Listener] +type=listener +service=CLI +protocol=maxscaled +socket=default + +[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 diff --git a/maxscale-system-test/mxs1926_killed_server.cpp b/maxscale-system-test/mxs1926_killed_server.cpp new file mode 100644 index 000000000..fa5aa0737 --- /dev/null +++ b/maxscale-system-test/mxs1926_killed_server.cpp @@ -0,0 +1,106 @@ +/** + * MXS-1926: LOAD DATA LOCAL INFILE interrupted by server shutdown + * + * https://jira.mariadb.org/browse/MXS-1926 + */ + +#include "testconnections.h" +#include +#include +#include +#include +#include + +using namespace std::chrono; + +typedef high_resolution_clock Clock; + +std::atomic ROWCOUNT{10000}; + +std::string create_tmpfile() +{ + char filename[] = "/tmp/data.csv.XXXXXX"; + int fd = mkstemp(filename); + std::ofstream file(filename); + close(fd); + + for (int i = 0; i < ROWCOUNT; i++) + { + file << "1, 2, 3, 4\n"; + } + + return filename; +} + +void tune_rowcount(TestConnections& test) +{ + milliseconds dur{1}; + test.tprintf("Tuning data size so that an insert takes 10 seconds"); + test.maxscales->connect(); + test.try_query(test.maxscales->conn_rwsplit[0], "SET sql_log_bin=0"); + + while (dur < seconds(10)) + { + std::string filename = create_tmpfile(); + + auto start = Clock::now(); + test.try_query(test.maxscales->conn_rwsplit[0], "LOAD DATA LOCAL INFILE '%s' INTO TABLE test.t1", + filename.c_str()); + auto end = Clock::now(); + dur = duration_cast(end - start); + test.try_query(test.maxscales->conn_rwsplit[0], "TRUNCATE TABLE test.t1", filename.c_str()); + + remove(filename.c_str()); + + int orig = ROWCOUNT; + ROWCOUNT = orig / dur.count() * 10000; + test.tprintf("Loading %d rows took %d ms, setting row count to %d", + orig, dur.count(), ROWCOUNT.load()); + } + + test.maxscales->disconnect(); +} + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + test.repl->connect(); + + // Create the table + execute_query(test.repl->nodes[0], "CREATE OR REPLACE TABLE test.t1 (a INT, b INT, c INT, d INT)"); + test.repl->sync_slaves(); + + // Tune the amount of data so that the loading will take approximately 15 seconds + tune_rowcount(test); + + std::string filename = create_tmpfile(); + + // Connect to MaxScale and load enough data so that we have + test.maxscales->connect(); + + // Disable replication of the LOAD DATA LOCAL INFILE + test.try_query(test.maxscales->conn_rwsplit[0], "SET sql_log_bin=0"); + + test.tprintf("Loading %d rows of data while stopping a slave", ROWCOUNT.load()); + std::thread thr([&]() + { + std::this_thread::sleep_for(milliseconds(10)); + test.repl->stop_node(3); + test.repl->start_node(3); + }); + test.try_query(test.maxscales->conn_rwsplit[0], "LOAD DATA LOCAL INFILE '%s' INTO TABLE test.t1", + filename.c_str()); + test.tprintf("Load complete"); + thr.join(); + + test.maxscales->disconnect(); + + // Cleanup + execute_query(test.repl->nodes[0], "DROP TABLE test.t1"); + test.repl->sync_slaves(); + test.repl->disconnect(); + + remove(filename.c_str()); + return test.global_result; +} From d9cc65cca7e64a59177eb450290e66c27b04b8e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Jun 2018 11:04:23 +0300 Subject: [PATCH 2/5] Enable mxs1824_double_cursor in 2.2 The test apparently started working when 10.3 was set as the default backend. --- maxscale-system-test/CMakeLists.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index a54243e61..a89006917 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -654,9 +654,6 @@ add_test_executable(mxs1808_long_data.cpp mxs1808_long_data replication LABELS r # https://jira.mariadb.org/browse/MXS-1824 add_test_executable(mxs1824_double_cursor.cpp mxs1824_double_cursor replication LABELS readwritesplit REPL_BACKEND) -# TODO: Remove this once the problem with the connector is resolved -set_tests_properties(mxs1824_double_cursor PROPERTIES WILL_FAIL TRUE) - # MXS-1831: No error on invalid monitor parameter alteration # https://jira.mariadb.org/browse/MXS-1831 add_test_executable(mxs1831_unknown_param.cpp mxs1831_unknown_param replication LABELS REPL_BACKEND) From d22a62c0ffecd380d6999a751e7d4d56b26db3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 18 Jun 2018 21:34:18 +0300 Subject: [PATCH 3/5] Use sleeps and waits with mxs1719 Going the belt-and-suspenders way of both sleeping and waiting for the moitor should make sure MaxScale has at least some time to start up, query the servers and do a single iteration of monitoring. --- maxscale-system-test/mxs1719.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/maxscale-system-test/mxs1719.cpp b/maxscale-system-test/mxs1719.cpp index efb634ea3..660ba5520 100644 --- a/maxscale-system-test/mxs1719.cpp +++ b/maxscale-system-test/mxs1719.cpp @@ -75,6 +75,7 @@ int main(int argc, char* argv[]) { if (test.maxscales->start() == 0) { + sleep(10); test.maxscales->wait_for_monitor(); if (test.maxscales->connect_rwsplit() == 0) From c49b6ada7dc4d6ee863eb7d80ac80be463481fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 21 Jun 2018 15:16:13 +0300 Subject: [PATCH 4/5] MXS-1932: Make it clear what is being tested The test was not very clear about what it does. Added comments to clarify what is done and why. Also fixed a typo. --- maxscale-system-test/mxs1932_hidden_cnf.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/maxscale-system-test/mxs1932_hidden_cnf.cpp b/maxscale-system-test/mxs1932_hidden_cnf.cpp index 4816cb369..922df5fac 100644 --- a/maxscale-system-test/mxs1932_hidden_cnf.cpp +++ b/maxscale-system-test/mxs1932_hidden_cnf.cpp @@ -16,6 +16,7 @@ int main(int argc, char** argv) TestConnections::skip_maxscale_start(true); TestConnections test(argc, argv); + // Create a file with a guaranteed bad configuration (turbochargers are not yet supported) ofstream cnf("hidden.cnf"); cnf << "[something]" << endl; cnf << "type=turbocharger" << endl; @@ -23,14 +24,18 @@ int main(int argc, char** argv) cnf << "speed=maximum" << endl; cnf.close(); + // Copy the configuration to MaxScale test.maxscales->copy_to_node_legacy("hidden.cnf", "~"); + + // Move it into the maxscale.cnf.d directory and make it a hidden file test.maxscales->ssh_node_f(0, true, "mkdir -p /etc/maxscale.cnf.d/;" "mv %s/hidden.cnf /etc/maxscale.cnf.d/.hidden.cnf;" "chown -R maxscale:maxscale /etc/maxscale.cnf.d/", test.maxscales->access_homedir[0]); - test.assert(test.maxscales->restart_maxscale() == 0, "Starting MaxScale should suceed"); + // Make sure the hidden configuration is not read and that MaxScale starts up + test.assert(test.maxscales->restart_maxscale() == 0, "Starting MaxScale should succeed"); test.maxscales->ssh_node_f(0, true, "rm -r /etc/maxscale.cnf.d/"); remove("hidden.cnf"); From e561c3995c7396cf3749ccdf6a3357d7dd32c856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 21 Jun 2018 14:43:15 +0300 Subject: [PATCH 5/5] Use correct write in Backend::execute_session_command Backend::execute_session_command would use the overridden write method instead of the Backend::write method that it intended to use. This caused session commands that did not expect a response to be in a state that expected a result. Also fixed RWBackend::write pass the response_type value to Backend::write. --- server/core/backend.cc | 19 +++++++++---------- .../routing/readwritesplit/rwsplitsession.cc | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/server/core/backend.cc b/server/core/backend.cc index edd1ef1cc..0a6f631d0 100644 --- a/server/core/backend.cc +++ b/server/core/backend.cc @@ -87,18 +87,18 @@ bool Backend::execute_session_command() CHK_DCB(m_dcb); - SessionCommandList::iterator iter = m_session_commands.begin(); - SessionCommand& sescmd = *(*iter); - GWBUF *buffer = sescmd.deep_copy_buffer(); + SSessionCommand& sescmd = m_session_commands.front(); + GWBUF *buffer = sescmd->deep_copy_buffer(); bool rval = false; - switch (sescmd.get_command()) + switch (sescmd->get_command()) { case MXS_COM_QUIT: case MXS_COM_STMT_CLOSE: /** These commands do not generate responses */ - rval = write(buffer, NO_RESPONSE); + rval = Backend::write(buffer, NO_RESPONSE); complete_session_command(); + ss_dassert(!is_waiting_result()); break; case MXS_COM_CHANGE_USER: @@ -109,12 +109,11 @@ bool Backend::execute_session_command() case MXS_COM_QUERY: default: - /** - * Mark session command buffer, it triggers writing - * MySQL command to protocol - */ + // TODO: Remove use of GWBUF_TYPE_SESCMD + //Mark session command buffer, it triggers writing MySQL command to protocol gwbuf_set_type(buffer, GWBUF_TYPE_SESCMD); - rval = write(buffer); + rval = Backend::write(buffer); + ss_dassert(is_waiting_result()); break; } diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index 35476403d..beeba1bbc 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/server/modules/routing/readwritesplit/rwsplitsession.cc @@ -96,7 +96,7 @@ bool RWBackend::write(GWBUF* buffer, response_type type) } } - return mxs::Backend::write(buffer); + return mxs::Backend::write(buffer, type); } bool RWBackend::consume_fetched_rows(GWBUF* buffer)