From 9f7a7e473e14fb632eff25e5f89e9282f6675d20 Mon Sep 17 00:00:00 2001 From: Andreas Kruger Date: Wed, 27 Mar 2019 10:04:40 +0100 Subject: [PATCH 1/5] Enable galeramon to track wsrep_desync, wsrep_ready, wsrep_sst_donor_rejects_queries and wsrep_reject_queries --- server/modules/monitor/galeramon/galeramon.cc | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index 1d3ceaa3b..10598c321 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/server/modules/monitor/galeramon/galeramon.cc @@ -141,7 +141,11 @@ void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) " ('wsrep_cluster_state_uuid'," " 'wsrep_cluster_size'," " 'wsrep_local_index'," - " 'wsrep_local_state')"; + " 'wsrep_local_state'," + " 'wsrep_desync'," + " 'wsrep_ready'," + " 'wsrep_sst_donor_rejects_queries'," + " 'wsrep_reject_queries')"; if (mxs_mysql_query(monitored_server->con, cluster_member) == 0 && (result = mysql_store_result(monitored_server->con)) != NULL) @@ -208,6 +212,42 @@ void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) info.local_state = atoi(row[1]); } + /* Node is in desync - lets take it offline */ + if (strcmp(row[0], "wsrep_desync") == 0) + { + if (strcasecmp(row[1],"YES") || strcasecmp(row[1],"ON") || strcasecmp(row[1],"1") || strcasecmp(row[1],"true")) + { + info.joined = 0; + } + } + + /* Node rejects queries - lets take it offline */ + if (strcmp(row[0], "wsrep_reject_queries") == 0) + { + if (strcasecmp(row[1],"ALL") || strcasecmp(row[1],"ALL_KILL")) + { + info.joined = 0; + } + } + + /* Node rejects queries - lets take it offline */ + if (strcmp(row[0], "wsrep_sst_donor_rejects_queries") == 0) + { + if (strcasecmp(row[1],"YES") || strcasecmp(row[1],"ON") || strcasecmp(row[1],"1") || strcasecmp(row[1],"true")) + { + info.joined = 0; + } + } + + /* Node is not ready - lets take it offline */ + if (strcmp(row[0], "wsrep_ready") == 0) + { + if (strcasecmp(row[1],"YES") || strcasecmp(row[1],"ON") || strcasecmp(row[1],"1") || strcasecmp(row[1],"true")) + { + info.joined = 0; + } + } + if (strcmp(row[0], "wsrep_cluster_state_uuid") == 0 && row[1] && *row[1]) { info.cluster_uuid = row[1]; From 7a5f11b752f60c7daa506ca9b0413ceabac8f500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Kr=C3=BCger?= Date: Tue, 16 Apr 2019 16:19:08 +0200 Subject: [PATCH 2/5] Fix wrong check for wsrep_ready wsrep_ready was check for ON/YES/1/true, but it has to be checked for OFF/NO/0/false as we are removing nodes, not joining. --- server/modules/monitor/galeramon/galeramon.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index 10598c321..f84ef38b6 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/server/modules/monitor/galeramon/galeramon.cc @@ -242,7 +242,7 @@ void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) /* Node is not ready - lets take it offline */ if (strcmp(row[0], "wsrep_ready") == 0) { - if (strcasecmp(row[1],"YES") || strcasecmp(row[1],"ON") || strcasecmp(row[1],"1") || strcasecmp(row[1],"true")) + if (strcasecmp(row[1],"NO") || strcasecmp(row[1],"OFF") || strcasecmp(row[1],"0") || strcasecmp(row[1],"false")) { info.joined = 0; } From 07ea6bd9ba16d669a6f88f09d91c56a9b8c5c17e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 23 Apr 2019 20:33:28 +0300 Subject: [PATCH 3/5] MXS-2450: Don't discard history if it's disabled If the session command history is not enabled, it shouldn't be discarded when a COM_CHANGE_USER is executed. --- maxscale-system-test/CMakeLists.txt | 3 ++ ...ale.cnf.template.mxs2450_change_user_crash | 49 +++++++++++++++++++ .../mxs2450_change_user_crash.cpp | 20 ++++++++ .../readwritesplit/rwsplit_session_cmd.cc | 3 +- 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100755 maxscale-system-test/cnf/maxscale.cnf.template.mxs2450_change_user_crash create mode 100644 maxscale-system-test/mxs2450_change_user_crash.cpp diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 11cf0fbf2..ba246796d 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -942,6 +942,9 @@ add_test_executable(mxs2326_hint_clone.cpp mxs2326_hint_clone mxs2326_hint_clone # MXS-2417: Ignore persisted configs with load_persisted_configs=false add_test_executable(mxs2417_ignore_persisted_cnf.cpp mxs2417_ignore_persisted_cnf mxs2417_ignore_persisted_cnf LABELS REPL_BACKEND) +# MXS-2450: Crash on COM_CHANGE_USER with disable_sescmd_history=true +add_test_executable(mxs2450_change_user_crash.cpp mxs2450_change_user_crash mxs2450_change_user_crash LABELS REPL_BACKEND) + ############################################ # BEGIN: binlogrouter and avrorouter tests # ############################################ diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mxs2450_change_user_crash b/maxscale-system-test/cnf/maxscale.cnf.template.mxs2450_change_user_crash new file mode 100755 index 000000000..38c574626 --- /dev/null +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mxs2450_change_user_crash @@ -0,0 +1,49 @@ +[maxscale] +threads=###threads### +log_info=1 + +[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 + +[MySQL Monitor] +type=monitor +module=mysqlmon +servers=server1,server2,server3,server4 +user=maxskysql +password=skysql +monitor_interval=1000 + +[RW Split Router] +type=service +router=readwritesplit +servers=server1,server2,server3,server4 +user=maxskysql +password=skysql +disable_sescmd_history=true + +[RW Split Listener] +type=listener +service=RW Split Router +protocol=MySQLClient +port=4006 diff --git a/maxscale-system-test/mxs2450_change_user_crash.cpp b/maxscale-system-test/mxs2450_change_user_crash.cpp new file mode 100644 index 000000000..a04223db4 --- /dev/null +++ b/maxscale-system-test/mxs2450_change_user_crash.cpp @@ -0,0 +1,20 @@ +/** + * MXS-2450: Crash on COM_CHANGE_USER with disable_sescmd_history=true + * https://jira.mariadb.org/browse/MXS-2450 + */ + +#include "testconnections.h" + +int main(int argc, char *argv[]) +{ + TestConnections test(argc, argv); + Connection conn = test.maxscales->rwsplit(); + test.expect(conn.connect(), "Connection failed: %s", conn.error()); + + for (int i = 0; i < 10; i++) + { + test.expect(conn.reset_connection(), "Connection reset failed: %s", conn.error()); + } + + return test.global_result; +} diff --git a/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc b/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc index e2d501f68..57963013a 100644 --- a/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc +++ b/server/modules/routing/readwritesplit/rwsplit_session_cmd.cc @@ -162,9 +162,10 @@ void RWSplitSession::process_sescmd_response(SRWBackend& backend, GWBUF** ppPack *ppPacket = NULL; } - if (m_expected_responses == 0 + if (m_expected_responses == 0 && !m_config.disable_sescmd_history && (command == MXS_COM_CHANGE_USER || command == MXS_COM_RESET_CONNECTION)) { + mxb_assert_message(!m_sescmd_list.empty(), "Must have stored session commands"); mxb_assert_message(m_slave_responses.empty(), "All responses should've been processed"); // This is the last session command to finish that resets the session state, reset the history MXS_INFO("Resetting session command history (length: %lu)", m_sescmd_list.size()); From 08bd7c99bee1c33fd85c7a80720064a1986d5de1 Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Fri, 26 Apr 2019 17:30:10 +0300 Subject: [PATCH 4/5] Add a possibility to run tests under callgrind Flag 'use_callgrind' make all maxscale-system-test run Maxscale under Valgrind with --tool=callgrind option --- maxscale-system-test/maxscales.cpp | 33 +++++++++++++++++++----- maxscale-system-test/maxscales.h | 8 +++++- maxscale-system-test/testconnections.cpp | 8 +++--- maxscale-system-test/testconnections.h | 5 ---- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/maxscale-system-test/maxscales.cpp b/maxscale-system-test/maxscales.cpp index 16894d35e..127a34c00 100644 --- a/maxscale-system-test/maxscales.cpp +++ b/maxscale-system-test/maxscales.cpp @@ -4,17 +4,16 @@ #include #include "envv.h" -Maxscales::Maxscales(const char *pref, const char *test_cwd, bool verbose, bool use_valgrind, +Maxscales::Maxscales(const char *pref, const char *test_cwd, bool verbose, std::string network_config) { strcpy(prefix, pref); this->verbose = verbose; - this->use_valgrind = use_valgrind; valgring_log_num = 0; strcpy(test_dir, test_cwd); this->network_config = network_config; read_env(); - if (use_valgrind) + if (this->use_valgrind) { for (int i = 0; i < N; i++) { @@ -61,6 +60,13 @@ int Maxscales::read_env() } } + use_valgrind = readenv_bool("use_valgrind", false); + use_callgrind = readenv_bool("use_callgrind", false); + if (use_callgrind) + { + use_valgrind = true; + } + return 0; } @@ -211,10 +217,23 @@ int Maxscales::start_maxscale(int m) int res; if (use_valgrind) { - 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); + if (use_callgrind) + { + res = ssh_node_f(m, false, + "sudo --user=maxscale valgrind -d " + "--log-file=/%s/valgrind%02d.log --trace-children=yes " + " --tool=callgrind --callgrind-out-file=/%s/callgrind%02d.log " + " /usr/bin/maxscale", + maxscale_log_dir[m], valgring_log_num, + maxscale_log_dir[m], valgring_log_num); + } + else + { + 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/maxscales.h b/maxscale-system-test/maxscales.h index b51ced8f2..491d51997 100644 --- a/maxscale-system-test/maxscales.h +++ b/maxscale-system-test/maxscales.h @@ -20,7 +20,7 @@ public: READCONN_SLAVE }; - Maxscales(const char *pref, const char *test_cwd, bool verbose, bool use_valgrind, + Maxscales(const char *pref, const char *test_cwd, bool verbose, std::string network_config); int read_env(); @@ -338,6 +338,12 @@ public: */ bool use_valgrind; + /** + * @brief use_callgrind if true Maxscale will be executed under Valgrind with + * --callgrind option + */ + bool use_callgrind; + /** * @brief valgring_log_num Counter for Maxscale restarts to avoid Valgrind log overwriting */ diff --git a/maxscale-system-test/testconnections.cpp b/maxscale-system-test/testconnections.cpp index 53df020f2..1f9838176 100644 --- a/maxscale-system-test/testconnections.cpp +++ b/maxscale-system-test/testconnections.cpp @@ -127,7 +127,6 @@ TestConnections::TestConnections(int argc, char* argv[]) , no_vm_revert(true) , threads(4) , use_ipv6(false) - , use_valgrind(false) { std::ios::sync_with_stdio(true); signal_set(SIGSEGV, sigfatal_handler); @@ -372,7 +371,7 @@ TestConnections::TestConnections(int argc, char* argv[]) galera = NULL; } - maxscales = new Maxscales("maxscale", test_dir, verbose, use_valgrind, network_config); + maxscales = new Maxscales("maxscale", test_dir, verbose, network_config); bool maxscale_ok = maxscales->check_nodes(); bool repl_ok = no_repl || repl_future.get(); @@ -502,7 +501,7 @@ TestConnections::~TestConnections() // galera->disable_ssl(); } - if (use_valgrind) + if (maxscales->use_valgrind) { // stop all Maxscales to get proper Valgrind logs for (int i = 0; i < maxscales->N; i++) @@ -665,7 +664,6 @@ void TestConnections::read_env() revert_snapshot_command = readenv("revert_snapshot_command", "mdbci snapshot revert --path-to-nodes %s --snapshot-name ", mdbci_config_name); no_vm_revert = readenv_bool("no_vm_revert", true); - use_valgrind = readenv_bool("use_valgrind", false); } void TestConnections::print_env() @@ -1459,7 +1457,7 @@ int TestConnections::find_connected_slave1(int m) int TestConnections::check_maxscale_processes(int m, int expected) { - const char* ps_cmd = use_valgrind ? + const char* ps_cmd = maxscales->use_valgrind ? "ps ax | grep valgrind | grep maxscale | grep -v grep | wc -l" : "ps -C maxscale | grep maxscale | wc -l"; diff --git a/maxscale-system-test/testconnections.h b/maxscale-system-test/testconnections.h index aebcb86aa..6b14dff5c 100644 --- a/maxscale-system-test/testconnections.h +++ b/maxscale-system-test/testconnections.h @@ -686,11 +686,6 @@ public: */ int call_mdbci(const char *options); - /** - * @brief use_valrind if true Maxscale will be executed under Valgrind - */ - bool use_valgrind; - /** * @brief resinstall_maxscales Remove Maxscale form all nodes and installs new ones * (to be used for run_test_snapshot) From dd188962cd99ee6caa908f8b7e1c2246135190ec Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Thu, 25 Apr 2019 10:30:29 +0300 Subject: [PATCH 5/5] MXS-2427 Check all hints when routing Now considers other routing hints if first one fails. The order is inverted compared to e.g. namedserver filter settings because of how routing hints are stored. If all hints are unsuccessful, route to any slave. --- .../readwritesplit/rwsplit_route_stmt.cc | 132 ++++++++---------- .../routing/readwritesplit/rwsplitsession.hh | 4 +- 2 files changed, 62 insertions(+), 74 deletions(-) diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index 91edc010c..662d91d9c 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -597,7 +597,7 @@ static inline bool rpl_lag_is_ok(SRWBackend& backend, int max_rlag) return max_rlag == MXS_RLAG_UNDEFINED || backend->server()->rlag <= max_rlag; } -SRWBackend RWSplitSession::get_hinted_backend(char* name) +SRWBackend RWSplitSession::get_hinted_backend(const char* name) { SRWBackend rval; @@ -735,15 +735,15 @@ SRWBackend RWSplitSession::get_last_used_backend() * * @param rses Pointer to router client session * @param btype Backend type - * @param name Name of the backend which is primarily searched. May be NULL. + * @param name Name of the requested backend. May be NULL if any name is accepted. * @param max_rlag Maximum replication lag * @param target The target backend * * @return True if a backend was found */ SRWBackend RWSplitSession::get_target_backend(backend_type_t btype, - char* name, - int max_rlag) + const char* name, + int max_rlag) { /** Check whether using target_node as target SLAVE */ if (m_target_node && session_trx_is_read_only(m_client->session)) @@ -752,13 +752,11 @@ SRWBackend RWSplitSession::get_target_backend(backend_type_t btype, } SRWBackend rval; - - if (name) /*< Choose backend by name from a hint */ + if (name) { - btype = BE_SLAVE; + // Choose backend by name from a hint rval = get_hinted_backend(name); } - else if (btype == BE_SLAVE) { rval = get_slave_backend(max_rlag); @@ -767,7 +765,6 @@ SRWBackend RWSplitSession::get_target_backend(backend_type_t btype, { rval = get_master_backend(); } - return rval; } @@ -808,82 +805,73 @@ int RWSplitSession::get_max_replication_lag() */ SRWBackend RWSplitSession::handle_hinted_target(GWBUF* querybuf, route_target_t route_target) { - char* named_server = NULL; - int rlag_max = MXS_RLAG_UNDEFINED; + const char rlag_hint_tag[] = "max_slave_replication_lag"; + const int comparelen = sizeof(rlag_hint_tag); + int config_max_rlag = get_max_replication_lag(); // From router configuration. + SRWBackend target; - HINT* hint = querybuf->hint; - - while (hint != NULL) + for (HINT* hint = querybuf->hint; !target && hint; hint = hint->next) { if (hint->type == HINT_ROUTE_TO_NAMED_SERVER) { - /** - * Set the name of searched - * backend server. - */ - named_server = (char*)hint->data; - MXS_INFO("Hint: route to server '%s'", named_server); - } - else if (hint->type == HINT_PARAMETER - && (strncasecmp((char*)hint->data, - "max_slave_replication_lag", - strlen("max_slave_replication_lag")) == 0)) - { - int val = (int)strtol((char*)hint->value, (char**)NULL, 10); - - if (val != 0 || errno == 0) + // Set the name of searched backend server. + const char* named_server = (char*)hint->data; + MXS_INFO("Hint: route to server '%s'.", named_server); + target = get_target_backend(BE_UNDEFINED, named_server, config_max_rlag); + if (!target) { - /** Set max. acceptable replication lag value for backend srv */ - rlag_max = val; - MXS_INFO("Hint: max_slave_replication_lag=%d", rlag_max); + // Target may differ from the requested name if the routing target is locked, e.g. by a trx. + // Target is null only if not locked and named server was not found or was invalid. + if (mxb_log_is_priority_enabled(LOG_INFO)) + { + char* status = nullptr; + for (const auto& a : m_backends) + { + if (strcmp(a->server()->name, named_server) == 0) + { + status = server_status(a->server()); + break; + } + } + MXS_INFO("Was supposed to route to named server %s but couldn't find the server in a " + "suitable state. Server state: %s", + named_server, status ? status : "Could not find server"); + MXS_FREE(status); + } + } + } + else if (hint->type == HINT_PARAMETER + && (strncasecmp((char*)hint->data, rlag_hint_tag, comparelen) == 0)) + { + const char* str_val = (char*)hint->value; + int hint_max_rlag = (int)strtol(str_val, (char**)NULL, 10); + if (hint_max_rlag != 0 || errno == 0) + { + MXS_INFO("Hint: %s=%d", rlag_hint_tag, hint_max_rlag); + target = get_target_backend(BE_SLAVE, nullptr, hint_max_rlag); + if (!target) + { + MXS_INFO("Was supposed to route to server with replication lag " + "at most %d but couldn't find such a slave.", hint_max_rlag); + } + } + else + { + MXS_ERROR("Hint: Could not parse value of %s: '%s' is not a valid number.", + rlag_hint_tag, str_val); } } - hint = hint->next; - } /*< while */ - - if (rlag_max == MXS_RLAG_UNDEFINED) /*< no rlag max hint, use config */ - { - rlag_max = get_max_replication_lag(); } - /** target may be master or slave */ - backend_type_t btype = route_target & TARGET_SLAVE ? BE_SLAVE : BE_MASTER; - - /** - * Search backend server by name or replication lag. - * If it fails, then try to find valid slave or master. - */ - SRWBackend target = get_target_backend(btype, named_server, rlag_max); - if (!target) { - if (TARGET_IS_NAMED_SERVER(route_target)) - { - char* status = nullptr; + // If no target so far, pick any available. TODO: should this be error instead? + // Erroring here is more appropriate when namedserverfilter allows setting multiple target types + // e.g. target=server1,->slave - for (const auto& a : m_backends) - { - if (strcmp(a->server()->name, named_server) == 0) - { - status = server_status(a->server()); - break; - } - } - - MXS_INFO("Was supposed to route to named server %s but couldn't find the server in a " - "suitable state. Server state: %s", named_server, - status ? status : "Could not find server"); - MXS_FREE(status); - } - else if (TARGET_IS_RLAG_MAX(route_target)) - { - MXS_INFO("Was supposed to route to server with " - "replication lag at most %d but couldn't " - "find such a slave.", - rlag_max); - } + backend_type_t btype = route_target & TARGET_SLAVE ? BE_SLAVE : BE_MASTER; + target = get_target_backend(btype, NULL, config_max_rlag); } - return target; } diff --git a/server/modules/routing/readwritesplit/rwsplitsession.hh b/server/modules/routing/readwritesplit/rwsplitsession.hh index 85ef72782..03ffa098a 100644 --- a/server/modules/routing/readwritesplit/rwsplitsession.hh +++ b/server/modules/routing/readwritesplit/rwsplitsession.hh @@ -189,11 +189,11 @@ private: bool route_stored_query(); void close_stale_connections(); - mxs::SRWBackend get_hinted_backend(char* name); + mxs::SRWBackend get_hinted_backend(const char* name); mxs::SRWBackend get_slave_backend(int max_rlag); mxs::SRWBackend get_master_backend(); mxs::SRWBackend get_last_used_backend(); - mxs::SRWBackend get_target_backend(backend_type_t btype, char* name, int max_rlag); + mxs::SRWBackend get_target_backend(backend_type_t btype, const char* name, int max_rlag); bool handle_target_is_all(route_target_t route_target, GWBUF* querybuf,