From f20005dddca87ba72a0b4aa63c99fe69be2d67a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 27 Sep 2017 12:54:07 +0300 Subject: [PATCH 1/3] Add missing parameters to `alter monitor` The `script_timeout` and `journal_max_age` parameters weren't handled in the monitor alteration code. Also added missing documentation to maxadmin help output for `alter monitor`. --- server/core/config_runtime.cc | 18 ++++++++++++++++++ server/core/monitor.cc | 5 +++++ server/modules/routing/debugcli/debugcmd.c | 15 +++++++++------ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 3089be706..96836294f 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -503,6 +503,24 @@ bool runtime_alter_monitor(MXS_MONITOR *monitor, const char *key, const char *va monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_ATTEMPTS, ival); } } + else if (strcmp(key, CN_JOURNAL_MAX_AGE) == 0) + { + long ival = get_positive_int(value); + if (ival) + { + valid = true; + monitorSetJournalMaxAge(monitor, ival); + } + } + else if (strcmp(key, CN_SCRIPT_TIMEOUT) == 0) + { + long ival = get_positive_int(value); + if (ival) + { + valid = true; + monitorSetScriptTimeout(monitor, ival); + } + } else { /** We're modifying module specific parameters and we need to stop the monitor */ diff --git a/server/core/monitor.cc b/server/core/monitor.cc index cccda7afb..a82bedebe 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -1489,6 +1489,8 @@ static bool create_monitor_config(const MXS_MONITOR *monitor, const char *filena dprintf(file, "%s=%d\n", CN_BACKEND_WRITE_TIMEOUT, monitor->write_timeout); dprintf(file, "%s=%d\n", CN_BACKEND_READ_TIMEOUT, monitor->read_timeout); dprintf(file, "%s=%d\n", CN_BACKEND_CONNECT_ATTEMPTS, monitor->connect_attempts); + dprintf(file, "%s=%ld\n", CN_JOURNAL_MAX_AGE, monitor->journal_max_age); + dprintf(file, "%s=%d\n", CN_SCRIPT_TIMEOUT, monitor->script_timeout); if (monitor->databases) { @@ -1516,6 +1518,8 @@ static bool create_monitor_config(const MXS_MONITOR *monitor, const char *filena CN_BACKEND_WRITE_TIMEOUT, CN_BACKEND_READ_TIMEOUT, CN_BACKEND_CONNECT_ATTEMPTS, + CN_JOURNAL_MAX_AGE, + CN_SCRIPT_TIMEOUT, CN_SERVERS }; @@ -1699,6 +1703,7 @@ json_t* monitor_parameters_to_json(const MXS_MONITOR* monitor) json_object_set_new(rval, CN_BACKEND_WRITE_TIMEOUT, json_integer(monitor->write_timeout)); json_object_set_new(rval, CN_BACKEND_CONNECT_ATTEMPTS, json_integer(monitor->connect_attempts)); json_object_set_new(rval, CN_JOURNAL_MAX_AGE, json_integer(monitor->journal_max_age)); + json_object_set_new(rval, CN_SCRIPT_TIMEOUT, json_integer(monitor->script_timeout)); /** Add custom module parameters */ const MXS_MODULE* mod = get_module(monitor->module_name, MODULE_MONITOR); diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index 988a790de..f7e1b0aaa 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -1584,12 +1584,15 @@ struct subcommand alteroptions[] = "KEY=VALUE List of `key=value` pairs separated by spaces\n" "\n" "All monitors support the following values for KEY:\n" - "user Username used when connecting to servers\n" - "password Password used when connecting to servers\n" - "monitor_interval Monitoring interval in milliseconds\n" - "backend_connect_timeout Server coneection timeout in seconds\n" - "backend_write_timeout Server write timeout in seconds\n" - "backend_read_timeout Server read timeout in seconds\n" + "user Username used when connecting to servers\n" + "password Password used when connecting to servers\n" + "monitor_interval Monitoring interval in milliseconds\n" + "backend_connect_timeout Server coneection timeout in seconds\n" + "backend_write_timeout Server write timeout in seconds\n" + "backend_read_timeout Server read timeout in seconds\n" + "backend_connect_attempts Number of re-connection attempts\n" + "journal_max_age Maximum age of server state journal\n" + "script_timeout Timeout in seconds for monitor scripts\n" "\n" "This will alter an existing parameter of a monitor. To remove parameters,\n" "pass an empty value for a key e.g. 'maxadmin alter monitor my-monitor my-key='\n" From 3922f7a901be887235252b63c24cea5010a92629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 27 Sep 2017 14:46:38 +0300 Subject: [PATCH 2/3] Enable tee filter tests for 2.2 Enabled the tests that were accidentally disabled and cleaned up some of the redundant code in bug649. --- maxscale-system-test/CMakeLists.txt | 18 +++++++-------- maxscale-system-test/bug649.cpp | 36 +++++------------------------ 2 files changed, 15 insertions(+), 39 deletions(-) diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 2d443e18a..cd8a3353d 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -121,7 +121,7 @@ add_test_executable(bug547.cpp bug547 replication LABELS readwritesplit REPL_BAC add_test_executable(bug681.cpp bug681 galera.bug681 LABELS readwritesplit GALERA_BACKEND) # Regression case for the bug "crash with tee filter" -#add_test_executable(bug643.cpp bug643 bug643 LABELS tee REPL_BACKEND) +add_test_executable(bug643.cpp bug643 bug643 LABELS tee REPL_BACKEND) # Regression case for the bug ""Different error messages from MariaDB and Maxscale" add_test_script(bug561.sh bug561.sh replication LABELS MySQLAuth REPL_BACKEND) @@ -166,13 +166,13 @@ add_test_executable(bug626.cpp bug626 replication LABELS MySQLAuth MySQLProtocol add_test_executable(bug634.cpp bug634 replication LABELS readwritesplit REPL_BACKEND) # Regression cases for several TEE filter hangs -#add_test_executable(bug645.cpp bug645 bug645 LABELS tee REPL_BACKEND) -#add_test_executable(bug645_1.cpp bug645_1 bug645_1 LABELS tee REPL_BACKEND) -#add_test_executable(bug649.cpp bug649 bug645 LABELS tee) -#add_test_executable(bug650.cpp bug650 bug650 LABELS tee REPL_BACKEND) +add_test_executable(bug645.cpp bug645 bug645 LABELS tee REPL_BACKEND) +add_test_executable(bug645_1.cpp bug645_1 bug645_1 LABELS tee REPL_BACKEND) +add_test_executable(bug649.cpp bug649 bug645 LABELS tee REPL_BACKEND) +add_test_executable(bug650.cpp bug650 bug650 LABELS tee REPL_BACKEND) # Heavy test for TEE filter -#add_test_script(bug648 sql_queries bug648 LABELS tee UNSTABLE HEAVY REPL_BACKEND) +add_test_script(bug648 sql_queries bug648 LABELS tee HEAVY REPL_BACKEND) # Crash when host name for some user in mysql.user is very long add_test_executable(bug653.cpp bug653 replication LABELS MySQLAuth MySQLProtocol REPL_BACKEND) @@ -181,7 +181,7 @@ add_test_executable(bug653.cpp bug653 replication LABELS MySQLAuth MySQLProtocol add_test_executable(bug654.cpp bug654 replication LABELS maxscale REPL_BACKEND) # Regression case for the bug "Tee filter: closing child session causes MaxScale to fail" -#add_test_executable(bug657.cpp bug657 bug657 LABELS tee REPL_BACKEND) +add_test_executable(bug657.cpp bug657 bug657 LABELS tee REPL_BACKEND) # Block backends (master or all slaves) and tries to connect Maxscale add_test_executable(bug658.cpp bug658 replication LABELS readwritesplit readconnroute maxscale REPL_BACKEND) @@ -193,7 +193,7 @@ add_test_executable(bug662.cpp bug662 bug662 LABELS readwritesplit readconnroute add_test_executable(bug664.cpp bug664 bug664 LABELS MySQLAuth MySQLProtocol) # TEE fileter: execute long sequence of queries ans session commands in the loop -#add_test_executable(bug670.cpp bug670 bug670 LABELS tee REPL_BACKEND) +add_test_executable(bug670.cpp bug670 bug670 LABELS tee REPL_BACKEND) # Regression case for the bug "MaxScale crashes if "Users table data" is empty and "show dbusers" is executed in maxadmin" add_test_executable(bug673.cpp bug673 bug673 LABELS MySQLAuth REPL_BACKEND) @@ -376,7 +376,7 @@ add_test_executable(mxs431.cpp mxs431 sharding LABELS schemarouter REPL_BACKEND add_test_executable(mxs47.cpp mxs47 replication LABELS MySQLProtocol LIGHT REPL_BACKEND) # Regression case for the bug "USE hangs when Tee filter uses matching" -#add_test_executable(mxs501_tee_usedb.cpp mxs501_tee_usedb mxs501 LABELS tee REPL_BACKEND) +add_test_executable(mxs501_tee_usedb.cpp mxs501_tee_usedb mxs501 LABELS tee REPL_BACKEND) # Open connection, execute 'change user', close connection in the loop add_test_executable(mxs548_short_session_change_user.cpp mxs548_short_session_change_user mxs548 LABELS MySQLProtocol REPL_BACKEND) diff --git a/maxscale-system-test/bug649.cpp b/maxscale-system-test/bug649.cpp index fbb95d594..6b3f44b24 100644 --- a/maxscale-system-test/bug649.cpp +++ b/maxscale-system-test/bug649.cpp @@ -53,22 +53,14 @@ int main(int argc, char *argv[]) int check_iret[threads_num]; Test = new TestConnections(argc, argv); - int time_to_run = (Test->smoke) ? 10 : 30; - Test->set_timeout(10); - - Test->tprintf("Connecting to RWSplit %s\n", Test->maxscale_IP); - Test->connect_rwsplit(); + int time_to_run = 10; + Test->set_timeout(60); Test->repl->connect(); - Test->tprintf("Drop t1 if exists\n"); - execute_query(Test->repl->nodes[0], "DROP TABLE IF EXISTS t1;"); - Test->tprintf("Create t1\n"); - Test->add_result(create_t1(Test->repl->nodes[0]), "t1 creation Failed\n"); + create_t1(Test->repl->nodes[0]); + Test->repl->sync_slaves(); Test->repl->close_connections(); - Test->stop_timeout(); - sleep(5); - create_insert_string(sql, 65000, 1); Test->tprintf("Creating query threads\n", time_to_run); for (int j = 0; j < threads_num; j++) @@ -90,14 +82,7 @@ int main(int argc, char *argv[]) Test->set_timeout(30); Test->tprintf("Trying query to RWSplit, expecting failure, but not a crash\n"); - if (execute_query_silent(Test->conn_rwsplit, (char *) "show processlist;") == 0) - { - Test->add_result(1, "Failure is expected, but query is ok\n"); - } - - Test->stop_timeout(); - sleep(time_to_run); - + test.try_query(Test->conn_rwsplit, "show processlist;"); Test->tprintf("Setup firewall back to allow mysql\n"); Test->repl->unblock_node(0); fflush(stdout); @@ -110,23 +95,14 @@ int main(int argc, char *argv[]) pthread_join(parall_traffic1[i], NULL); Test->tprintf("exit %d\n", i); } - Test->stop_timeout(); - sleep(5); Test->set_timeout(20); Test->tprintf("Checking Maxscale is alive\n"); Test->check_maxscale_alive(); - Test->set_timeout(20); - Test->tprintf("Reconnecting to RWSplit ...\n"); - Test->connect_rwsplit(); - Test->tprintf(" ... and trying query\n"); - Test->try_query(Test->conn_rwsplit, (char *) "show processlist;"); - Test->close_rwsplit(); - /** Clean up */ Test->repl->connect(); - execute_query(Test->repl->nodes[0], "DROP TABLE IF EXISTS t1;"); + execute_query(Test->repl->nodes[0], "DROP TABLE t1;"); int rval = Test->global_result; delete Test; From 39c19e1bb905caf5f7f289c66c1ee39dbca63bea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 27 Sep 2017 18:12:46 +0300 Subject: [PATCH 3/3] Fix memory leak on loading of users If the new format users are loaded, the loaded JSON object was never freed. --- server/core/adminusers.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/adminusers.cc b/server/core/adminusers.cc index 17e9d2868..8acd6902c 100644 --- a/server/core/adminusers.cc +++ b/server/core/adminusers.cc @@ -290,6 +290,7 @@ static USERS* load_users(const char *fname) { /** New format users */ rval = users_from_json(json); + json_decref(json); } else {