From 4167e88719d53229ecc1afb0f6adc991019abe4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 29 Mar 2018 18:00:12 +0300 Subject: [PATCH] MXS-1751: Fix crash with available_when_donor=true The `MYSQL_ROW row` variable was being overwritten by the extra query done by the SST method detection code. Moving it into its own function prevents this and makes the code significantly easier to comprehend. Added a test case that reproduced the problem (MaxScale crashed) and verifies that the patch fixes the problem. --- maxscale-system-test/CMakeLists.txt | 4 ++ ...emplate.mxs1751_available_when_donor_crash | 53 ++++++++++++++ .../mxs1751_available_when_donor_crash.cpp | 30 ++++++++ server/modules/monitor/galeramon/galeramon.c | 69 +++++++++++-------- 4 files changed, 128 insertions(+), 28 deletions(-) create mode 100755 maxscale-system-test/cnf/maxscale.cnf.template.mxs1751_available_when_donor_crash create mode 100644 maxscale-system-test/mxs1751_available_when_donor_crash.cpp diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 0d39c0fc8..35c7c803b 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -621,6 +621,10 @@ add_test_executable(mxs1713_lots_of_databases.cpp mxs1713_lots_of_databases mxs1 # https://jira.mariadb.org/browse/MXS-1731 add_test_executable(mxs1731_old_persisted_config.cpp mxs1731_old_persisted_config replication LABELS REPL_BACKEND) +# MXS-1751: Maxscale crashes when certain config is in play (with nodes down) +# https://jira.mariadb.org/browse/MXS-1751 +add_test_executable(mxs1751_available_when_donor_crash.cpp mxs1751_available_when_donor_crash mxs1751_available_when_donor_crash LABELS GALERA_BACKEND) + # 'namedserverfilter' test add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND) diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mxs1751_available_when_donor_crash b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1751_available_when_donor_crash new file mode 100755 index 000000000..39cc92b9c --- /dev/null +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1751_available_when_donor_crash @@ -0,0 +1,53 @@ +[maxscale] +threads=###threads### + +[Galera Monitor] +type=monitor +module=galeramon +servers=server1,server2,server3 +user=maxskysql +passwd=skysql +monitor_interval=100 +available_when_donor=true + +[RW Split Router] +type=service +router=readwritesplit +servers=server1,server2,server3 +user=maxskysql +passwd=skysql +master_accept_reads=true + +[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=###galera_server_IP_1### +port=###galera_server_port_1### +protocol=MySQLBackend + +[server2] +type=server +address=###galera_server_IP_2### +port=###galera_server_port_2### +protocol=MySQLBackend + +[server3] +type=server +address=###galera_server_IP_3### +port=###galera_server_port_3### +protocol=MySQLBackend diff --git a/maxscale-system-test/mxs1751_available_when_donor_crash.cpp b/maxscale-system-test/mxs1751_available_when_donor_crash.cpp new file mode 100644 index 000000000..b47647c1c --- /dev/null +++ b/maxscale-system-test/mxs1751_available_when_donor_crash.cpp @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2016 MariaDB Corporation Ab + * + * Use of this software is governed by the Business Source License included + * in the LICENSE.TXT file and at www.mariadb.com/bsl11. + * + * Change Date: 2020-01-01 + * + * On the date above, in accordance with the Business Source License, use + * of this software will be governed by version 2 or later of the General + * Public License. + */ + +#include "testconnections.h" + +int main(int argc, char* argv[]) +{ + TestConnections test(argc, argv); + + for (int i = 0; i < 2; i++) + { + test.galera->stop_node(0); + test.galera->stop_node(1); + test.galera->start_node(1); + test.galera->start_node(0); + test.galera->fix_replication(); + } + + return test.global_result; +} diff --git a/server/modules/monitor/galeramon/galeramon.c b/server/modules/monitor/galeramon/galeramon.c index 0cc60162f..a03e4f28a 100644 --- a/server/modules/monitor/galeramon/galeramon.c +++ b/server/modules/monitor/galeramon/galeramon.c @@ -260,6 +260,41 @@ static json_t* diagnostics_json(const MXS_MONITOR *mon) return rval; } +static bool using_xtrabackup(MXS_MONITORED_SERVER *database, const char* server_string) +{ + bool rval = false; + MYSQL_RES* result; + + if (mxs_mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0 + && (result = mysql_store_result(database->con)) != NULL) + { + if (mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + MXS_ERROR("Unexpected result for \"SHOW VARIABLES LIKE " + "'wsrep_sst_method'\". Expected 2 columns." + " MySQL Version: %s", server_string); + } + + MYSQL_ROW row; + + while ((row = mysql_fetch_row(result))) + { + if (row[1] && strncmp(row[1], "xtrabackup", 10) == 0) + { + rval = true; + } + } + mysql_free_result(result); + } + else + { + mon_report_query_error(database); + } + + return rval; +} + /** * Monitor an individual server. Does not deal with the setting of master or * slave bits, except for clearing them when a server is not joined to the @@ -273,8 +308,7 @@ monitorDatabase(MXS_MONITOR *mon, MXS_MONITORED_SERVER *database) { GALERA_MONITOR* handle = (GALERA_MONITOR*) mon->handle; MYSQL_ROW row; - MYSQL_RES *result, *result2; - int isjoined = 0; + MYSQL_RES *result; char *server_string; /* Don't even probe server flagged as in maintenance */ @@ -364,6 +398,8 @@ monitorDatabase(MXS_MONITOR *mon, MXS_MONITORED_SERVER *database) info.local_index = local_index; } + ss_dassert(row[0] && row[1]); + if (strcmp(row[0], "wsrep_local_state") == 0) { if (strcmp(row[1], "4") == 0) @@ -371,33 +407,10 @@ monitorDatabase(MXS_MONITOR *mon, MXS_MONITORED_SERVER *database) info.joined = 1; } /* Check if the node is a donor and is using xtrabackup, in this case it can stay alive */ - else if (strcmp(row[1], "2") == 0 && handle->availableWhenDonor == 1) + else if (strcmp(row[1], "2") == 0 && handle->availableWhenDonor == 1 && + using_xtrabackup(database, server_string)) { - if (mxs_mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0 - && (result2 = mysql_store_result(database->con)) != NULL) - { - if (mysql_field_count(database->con) < 2) - { - mysql_free_result(result); - mysql_free_result(result2); - MXS_ERROR("Unexpected result for \"SHOW VARIABLES LIKE " - "'wsrep_sst_method'\". Expected 2 columns." - " MySQL Version: %s", server_string); - return; - } - while ((row = mysql_fetch_row(result2))) - { - if (strncmp(row[1], "xtrabackup", 10) == 0) - { - info.joined = 1; - } - } - mysql_free_result(result2); - } - else - { - mon_report_query_error(database); - } + info.joined = 1; } else {