From c83868936a4c66bde242ec9e9b8c86b9da62b53b Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Fri, 8 Mar 2019 11:47:11 +0200 Subject: [PATCH 01/10] fix start_maxscale for valgrind case --- maxscale-system-test/maxscales.cpp | 6 +++++- maxscale-system-test/testconnections.cpp | 14 +++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/maxscale-system-test/maxscales.cpp b/maxscale-system-test/maxscales.cpp index 6ba1b30b9..e7aadbf58 100644 --- a/maxscale-system-test/maxscales.cpp +++ b/maxscale-system-test/maxscales.cpp @@ -247,7 +247,11 @@ int Maxscales::restart_maxscale(int m) if (use_valgrind) { res = stop_maxscale(m); - res += start_maxscale(m); + res = ssh_node_f(m, false, + "sudo --user=maxscale valgrind --leak-check=full --show-leak-kinds=all " + "--log-file=/%s/valgrind%02d.log --trace-children=yes " + "--track-origins=yes /usr/bin/maxscale", maxscale_log_dir[m], valgring_log_num); + valgring_log_num++; } else { diff --git a/maxscale-system-test/testconnections.cpp b/maxscale-system-test/testconnections.cpp index 56052b4c5..cd50061e8 100644 --- a/maxscale-system-test/testconnections.cpp +++ b/maxscale-system-test/testconnections.cpp @@ -776,12 +776,16 @@ void TestConnections::init_maxscale(int m) true, "cp maxscale.cnf %s;" "iptables -F INPUT;" - "rm -rf %s/*.log /tmp/core* /dev/shm/* /var/lib/maxscale/maxscale.cnf.d/ /var/lib/maxscale/*;" - "%s" - "maxctrl api get maxscale/debug/monitor_wait", + "rm -rf %s/*.log /tmp/core* /dev/shm/* /var/lib/maxscale/maxscale.cnf.d/ /var/lib/maxscale/*;", maxscales->maxscale_cnf[m], - maxscales->maxscale_log_dir[m], - maxscale::start ? "service maxscale restart;" : ""); + maxscales->maxscale_log_dir[m]); + if (maxscale::start) + { + maxscales->restart_maxscale(m); + maxscales->ssh_node_f(m, + true, + "maxctrl api get maxscale/debug/monitor_wait"); + } } void TestConnections::copy_one_mariadb_log(int i, std::string filename) From 71a3cde44144d0b10f4a52b8971d63a37507e7fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 Mar 2019 20:28:09 +0200 Subject: [PATCH 02/10] MXS-2373: Fix filter serialization The module of a filter was ignored as it wasn't in the list of expected module parameters. --- server/core/filter.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/core/filter.cc b/server/core/filter.cc index 77718419d..14c04884c 100644 --- a/server/core/filter.cc +++ b/server/core/filter.cc @@ -540,12 +540,13 @@ static bool create_filter_config(const SFilterDef& filter, const char* filename) dprintf(file, "[%s]\n", filter->name.c_str()); dprintf(file, "%s=%s\n", CN_TYPE, CN_FILTER); + dprintf(file, "%s=%s\n", CN_MODULE, filter->module.c_str()); const MXS_MODULE* mod = get_module(filter->module.c_str(), NULL); mxb_assert(mod); MXS_MODULE_PARAM no_common_params = {}; - dump_param_list(file, filter->parameters, {CN_TYPE}, &no_common_params, mod->parameters); + dump_param_list(file, filter->parameters, {CN_TYPE, CN_MODULE}, &no_common_params, mod->parameters); close(file); return true; From 7db87784accbaf0e5c17042232165f05b6c55878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Mar 2019 07:45:59 +0200 Subject: [PATCH 03/10] Deliver hangups only to valid DCBs If a DCB was closed and a hangup event was sent to it via dcb_hangup_foreach shortly after it was closed, the DCB would still receive it even if it was closed. To prevent this, events must only be delivered to DCBs if they haven't been closed. --- server/core/backend.cc | 4 ++-- server/core/dcb.cc | 3 +-- server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/core/backend.cc b/server/core/backend.cc index d54fdeb9e..348ac5329 100644 --- a/server/core/backend.cc +++ b/server/core/backend.cc @@ -47,7 +47,7 @@ Backend::~Backend() void Backend::close(close_type type) { - mxb_assert(m_dcb->n_close == 0); + mxb_assert(m_dcb && m_dcb->n_close == 0); if (!m_closed) { @@ -180,7 +180,7 @@ void Backend::set_state(backend_state state) bool Backend::connect(MXS_SESSION* session, SessionCommandList* sescmd) { - mxb_assert(!in_use()); + mxb_assert(!in_use() && m_dcb == nullptr); bool rval = false; if ((m_dcb = dcb_connect(m_backend->server, session, m_backend->server->protocol))) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index eb07a7ad7..52d9b248b 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2041,8 +2041,7 @@ static void dcb_hangup_foreach_worker(MXB_WORKER* worker, struct server* server) for (DCB* dcb = this_unit.all_dcbs[id]; dcb; dcb = dcb->thread.next) { - if (dcb->state == DCB_STATE_POLLING && dcb->server - && dcb->server == server) + if (dcb->state == DCB_STATE_POLLING && dcb->server && dcb->server == server && dcb->n_close == 0) { dcb->flags |= DCBF_HUNG; dcb->func.hangup(dcb); diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc index 2236f7da8..9de1843c1 100644 --- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc +++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc @@ -1354,6 +1354,7 @@ static int gw_error_backend_event(DCB* dcb) */ static int gw_backend_hangup(DCB* dcb) { + mxb_assert(dcb->n_close == 0); MXS_SESSION* session = dcb->session; if (dcb->persistentstart) From 5005facfb150e3e17ca42864f28addf5a6e8d0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Mar 2019 07:48:34 +0200 Subject: [PATCH 04/10] Always dump the full configuration Always storing runtime configuration changes prevents problems when the change causes another parameter to change. One example of this is transaction_replay that implicitly enables other parameters. --- server/core/config.cc | 36 ++++-------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/server/core/config.cc b/server/core/config.cc index d3d395a08..adbf12f1a 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -4954,36 +4954,6 @@ MXS_CONFIG_PARAMETER* ParamList::params() } } -void dump_if_changed(const MXS_MODULE_PARAM* params, - int file, - const std::string& key, - const std::string& value) -{ - for (int i = 0; params[i].name; i++) - { - if (params[i].name == key) - { - /** - * This detects only exact matches, not ones that are logically equivalent - * but lexicographically different e.g. 1 and true. This might not - * be a bad thing: it'll distinct user defined values from defaults. - */ - - if (!params[i].default_value || value != params[i].default_value) - { - if (dprintf(file, "%s=%s\n", key.c_str(), value.c_str()) == -1) - { - MXS_ERROR("Failed to serialize service value: %d, %s", - errno, - mxs_strerror(errno)); - } - } - - break; - } - } -} - void dump_param_list(int file, MXS_CONFIG_PARAMETER* list, const std::unordered_set& ignored, @@ -4994,8 +4964,10 @@ void dump_param_list(int file, { if (ignored.count(p->name) == 0 && *p->value) { - dump_if_changed(common_params, file, p->name, p->value); - dump_if_changed(module_params, file, p->name, p->value); + if (dprintf(file, "%s=%s\n", p->name, p->value) == -1) + { + MXS_ERROR("Failed to serialize service value: %d, %s", errno, mxs_strerror(errno)); + } } } } From 247e558ffa37acbcbe8ac1e0c7067d330bd36058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Mar 2019 09:51:49 +0200 Subject: [PATCH 05/10] Fix tls-model exposure to other modules The flag was propagated to other modules that depend on it. --- server/core/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/CMakeLists.txt b/server/core/CMakeLists.txt index 25c221525..9a4e6f2f7 100644 --- a/server/core/CMakeLists.txt +++ b/server/core/CMakeLists.txt @@ -83,7 +83,7 @@ endif() # Using initial-exec instead of the default global-dynamic tls-model # reduces the cost of using thread-local variables in dynamic libraries. -target_compile_options(maxscale-common PUBLIC "-ftls-model=initial-exec") +target_compile_options(maxscale-common PRIVATE "-ftls-model=initial-exec") add_dependencies(maxscale-common pcre2 connector-c libmicrohttpd jansson maxbase) set_target_properties(maxscale-common PROPERTIES VERSION "1.0.0") From 7f27db02a84bd98e812281c51721539058b3f615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Mar 2019 14:00:47 +0200 Subject: [PATCH 06/10] Fix global retain_last_statements The default value was wrong. --- server/core/config.cc | 2 +- server/core/service.cc | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/server/core/config.cc b/server/core/config.cc index adbf12f1a..c4e4d0385 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -313,7 +313,7 @@ const MXS_MODULE_PARAM config_service_params[] = {CN_LOG_AUTH_WARNINGS, MXS_MODULE_PARAM_BOOL, "true"}, {CN_RETRY_ON_FAILURE, MXS_MODULE_PARAM_BOOL, "true"}, {CN_SESSION_TRACK_TRX_STATE, MXS_MODULE_PARAM_BOOL, "false"}, - {CN_RETAIN_LAST_STATEMENTS, MXS_MODULE_PARAM_COUNT, "0"}, + {CN_RETAIN_LAST_STATEMENTS, MXS_MODULE_PARAM_INT, "-1"}, {NULL} }; diff --git a/server/core/service.cc b/server/core/service.cc index e73dbd5b9..b7ba9947f 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -199,15 +199,7 @@ Service::Service(const std::string& service_name, log_auth_warnings = config_get_bool(params, CN_LOG_AUTH_WARNINGS); strip_db_esc = config_get_bool(params, CN_STRIP_DB_ESC); session_track_trx_state = config_get_bool(params, CN_SESSION_TRACK_TRX_STATE); - - if (config_get_param(params, CN_RETAIN_LAST_STATEMENTS)) - { - retain_last_statements = config_get_integer(params, CN_RETAIN_LAST_STATEMENTS); - } - else - { - retain_last_statements = -1; // Indicates that it has not been set. - } + retain_last_statements = config_get_integer(params, CN_RETAIN_LAST_STATEMENTS); /** * At service start last update is set to config->users_refresh_time seconds earlier. From 5c5c6630bf0aff78e2fcd07eaacfd075d9194b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Mar 2019 16:57:38 +0200 Subject: [PATCH 07/10] Fix retain_last_statements If multiple statements were stored in a single buffer only one of them would get registered. --- server/modules/protocol/MySQL/mariadbclient/mysql_client.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index c1302ed0c..b6ea0b6c5 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -1118,8 +1118,6 @@ static int gw_read_normal_data(DCB* dcb, GWBUF* read_buffer, int nbytes_read) // is thread and not session specific. qc_set_sql_mode(static_cast(session->client_protocol_data)); } - - session_retain_statement(session, read_buffer); } /** Update the current protocol command being executed */ else if (!process_client_commands(dcb, nbytes_read, &read_buffer)) @@ -1671,6 +1669,7 @@ static int route_by_statement(MXS_SESSION* session, uint64_t capabilities, GWBUF { // TODO: Do this only when RCAP_TYPE_CONTIGUOUS_INPUT is requested packetbuf = gwbuf_make_contiguous(packetbuf); + session_retain_statement(session, packetbuf); MySQLProtocol* proto = (MySQLProtocol*)session->client_dcb->protocol; From 710e5df27ba5412db6912dc3309d239c0bda9e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Mar 2019 18:09:01 +0200 Subject: [PATCH 08/10] MXS-2365: Fix classification of queued queries Queries in the query queue need to be explicitly parsed since they are stored in a single buffer and thus share the query classification information. In the next major version this should be changed into an array of individual buffers instead of a shared buffer. --- server/modules/routing/readwritesplit/rwsplitsession.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/modules/routing/readwritesplit/rwsplitsession.cc b/server/modules/routing/readwritesplit/rwsplitsession.cc index 76d53d293..c0627f75c 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.cc +++ b/server/modules/routing/readwritesplit/rwsplitsession.cc @@ -223,7 +223,7 @@ bool RWSplitSession::route_stored_query() * to wait for a response before attempting another reroute */ while (m_query_queue) { - MXS_INFO("Routing stored queries"); + MXS_INFO(">>> Routing stored queries"); GWBUF* query_queue = modutil_get_next_MySQL_packet(&m_query_queue); query_queue = gwbuf_make_contiguous(query_queue); mxb_assert(query_queue); @@ -241,6 +241,9 @@ bool RWSplitSession::route_stored_query() GWBUF* temp_storage = m_query_queue; m_query_queue = NULL; + // The query needs to be explicitly parsed as it was processed multiple times + qc_parse(query_queue, QC_COLLECT_ALL); + // TODO: Move the handling of queued queries to the client protocol // TODO: module where the command tracking is done automatically. uint8_t cmd = mxs_mysql_get_command(query_queue); @@ -252,6 +255,8 @@ bool RWSplitSession::route_stored_query() MXS_ERROR("Failed to route queued query."); } + MXS_INFO("<<< Stored queries routed"); + if (m_query_queue == NULL) { /** Query successfully routed and no responses are expected */ From 160b4e6e05e2cb9a2fa4ed6de203b4be754e4a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 Mar 2019 04:33:54 +0200 Subject: [PATCH 09/10] MXS-2368: Fix reading of non-tty input The password input only worked if stdin was a TTY. This was caused by the fact that the readline-sync library only worked for TTYs. --- maxctrl/lib/common.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/maxctrl/lib/common.js b/maxctrl/lib/common.js index 379508d42..4003ac19c 100644 --- a/maxctrl/lib/common.js +++ b/maxctrl/lib/common.js @@ -50,9 +50,14 @@ module.exports = function() { // No password given, ask it from the command line if (argv.p == '') { - argv.p = readlineSync.question('Enter password: ', { - hideEchoBack: true - }) + if (process.stdin.isTTY) { + argv.p = readlineSync.question('Enter password: ', { + hideEchoBack: true + }) + } else { + var line = fs.readFileSync(0) + argv.p = line.toString().trim() + } } // Split the hostnames, separated by commas From c132125d555aff552f41cb0f609fa61322e7d11b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Sat, 9 Mar 2019 17:16:37 +0200 Subject: [PATCH 10/10] Fix log truncation Syslog wasn't truncated which caused massive disk space usage when the full test set was run. --- maxscale-system-test/mariadb_nodes.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/maxscale-system-test/mariadb_nodes.cpp b/maxscale-system-test/mariadb_nodes.cpp index 464fb4e1d..ffd314f64 100644 --- a/maxscale-system-test/mariadb_nodes.cpp +++ b/maxscale-system-test/mariadb_nodes.cpp @@ -1146,13 +1146,13 @@ int Mariadb_nodes::truncate_mariadb_logs() int local_result = 0; for (int node = 0; node < N; node++) { - char sys[1024]; if (strcmp(IP[node], "127.0.0.1") != 0) { - sprintf(sys, - "ssh -i %s -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o LogLevel=quiet %s@%s 'sudo truncate /var/lib/mysql/*.err --size 0;sudo rm -f /etc/my.cnf.d/binlog_enc*\' &", - sshkey[node], access_user[node], IP[node]); - local_result += system(sys); + local_result += ssh_node_f(node, true, + "truncate -s 0 /var/lib/mysql/*.err;" + "truncate -s 0 /var/log/syslog;" + "truncate -s 0 /var/log/messages;" + "rm -f /etc/my.cnf.d/binlog_enc*;"); } } return local_result;