From 7a8c30751587ff530e1ffff57e66530a2e235186 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 19:23:43 +0300 Subject: [PATCH 01/12] MXS-171: https://mariadb.atlassian.net/browse/MXS-171 Added option which allows the master server to be used for reads. --- Documentation/routers/ReadWriteSplit.md | 7 +++++++ server/modules/include/readwritesplit.h | 1 + server/modules/routing/readwritesplit/readwritesplit.c | 7 ++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/routers/ReadWriteSplit.md b/Documentation/routers/ReadWriteSplit.md index 577bc2e36..e58367789 100644 --- a/Documentation/routers/ReadWriteSplit.md +++ b/Documentation/routers/ReadWriteSplit.md @@ -93,6 +93,13 @@ disable_sescmd_history=true disable_slave_recovery=true ``` +**`master_accept_reads`** allows the master server to be used for reads. This is a useful option to enable if you are using a small number of servers and wish to use the master for reads as well. + +``` +# Use the master for reads +master_accept_reads=true +``` + ## Limitations In Master-Slave replication cluster also read-only queries are routed to master too in the following situations: diff --git a/server/modules/include/readwritesplit.h b/server/modules/include/readwritesplit.h index 9aa55016f..c78cde5b4 100644 --- a/server/modules/include/readwritesplit.h +++ b/server/modules/include/readwritesplit.h @@ -252,6 +252,7 @@ typedef struct rwsplit_config_st { int rw_max_sescmd_history_size; bool disable_sescmd_hist; bool disable_slave_recovery; + bool master_reads; /*< Use master for reads */ } rwsplit_config_t; diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b9beb80d0..7b4a808c3 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1244,7 +1244,8 @@ static bool get_dcb( SERVER_IS_SLAVE(b->backend_server) && (max_rlag == MAX_RLAG_UNDEFINED || (b->backend_server->rlag != MAX_RLAG_NOT_AVAILABLE && - b->backend_server->rlag <= max_rlag))) + b->backend_server->rlag <= max_rlag)) && + !rses->rses_config.master_reads) { /** found slave */ candidate_bref = &backend_ref[i]; @@ -4589,6 +4590,10 @@ static void rwsplit_process_router_options( { router->rwsplit_config.disable_slave_recovery = config_truth_value(value); } + else if(strcmp(options[i],"master_accept_reads") == 0) + { + router->rwsplit_config.master_reads = config_truth_value(value); + } } } /*< for */ } From 05991384a6f7ac9bef6d9d04ab165cb392dcfd41 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 27 Jun 2015 09:51:59 +0300 Subject: [PATCH 02/12] Added missing removal of systemd files to the postinstall script. --- etc/postrm.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etc/postrm.in b/etc/postrm.in index 65c0f7116..7488ebe46 100755 --- a/etc/postrm.in +++ b/etc/postrm.in @@ -3,7 +3,9 @@ if [ "$1" -eq 0 ] then rm -f /etc/init.d/maxscale rm -f /etc/ld.so.conf.d/maxscale.conf + rm -f /usr/lib/systemd/system/maxscale.service else + # Copy and rename config from old location if [ -f "/usr/local/mariadb-maxscale/etc/MaxScale.cnf" ] then cp "/usr/local/mariadb-maxscale/etc/MaxScale.cnf" "/etc/maxscale.cnf" From 113fb4c33bb6ee07c13821b8472b2e797e95ceff Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 28 Jun 2015 08:43:05 +0300 Subject: [PATCH 03/12] Fix to MXS-209: https://mariadb.atlassian.net/browse/MXS-209 Added missing checks for proper column count on query result. --- server/modules/monitor/galeramon.c | 15 +++++++++++ server/modules/monitor/mmmon.c | 35 +++++++++++++++++++++++--- server/modules/monitor/mysql_mon.c | 23 +++++++++++++++++ server/modules/monitor/ndbclustermon.c | 14 +++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index 06a6e6de8..e1ddc1217 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -372,6 +372,13 @@ char *server_string; if (mysql_query(database->con, "SHOW STATUS LIKE 'wsrep_local_state'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_state'\""); + return; + } + while ((row = mysql_fetch_row(result))) { if (strcmp(row[1], "4") == 0) @@ -398,6 +405,14 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long local_index = -1; + + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_index'\""); + return; + } + while ((row = mysql_fetch_row(result))) { local_index = strtol(row[1], NULL, 10); diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 545a77ca2..970fd0d31 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -366,7 +366,14 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long server_id = -1; - + + if(mysql_field_count(database->con) != 1) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + return; + } + while ((row = mysql_fetch_row(result))) { server_id = strtol(row[0], NULL, 10); @@ -392,7 +399,15 @@ char *server_string; { int i = 0; long master_id = -1; - + + if(mysql_field_count(database->con) < 42) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ @@ -431,7 +446,15 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long master_id = -1; - + + if(mysql_field_count(database->con) < 40) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW SLAVE STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ @@ -463,6 +486,12 @@ char *server_string; if (mysql_query(database->con, "SHOW GLOBAL VARIABLES LIKE 'read_only'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW GLOBAL VARIABLES LIKE 'read_only'\""); + return; + } while ((row = mysql_fetch_row(result))) { diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index e310e2ede..4c5f57ab6 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -408,6 +408,12 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long server_id = -1; + if(mysql_field_count(database->con) != 1) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + return; + } while ((row = mysql_fetch_row(result))) { server_id = strtol(row[0], NULL, 10); @@ -433,6 +439,15 @@ char *server_string; { int i = 0; long master_id = -1; + + if(mysql_field_count(database->con) < 42) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ @@ -471,6 +486,14 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long master_id = -1; + if(mysql_field_count(database->con) < 40) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW SLAVE STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ diff --git a/server/modules/monitor/ndbclustermon.c b/server/modules/monitor/ndbclustermon.c index 9561e275e..ed218d43e 100644 --- a/server/modules/monitor/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon.c @@ -323,6 +323,13 @@ char *server_string; if (mysql_query(database->con, "SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'\""); + return; + } + while ((row = mysql_fetch_row(result))) { if (atoi(row[1]) > 0) @@ -335,6 +342,13 @@ char *server_string; if (mysql_query(database->con, "SHOW STATUS LIKE 'Ndb_cluster_node_id'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_cluster_node_id'\""); + return; + } + long cluster_node_id = -1; while ((row = mysql_fetch_row(result))) { From 5c7a30e9fe326dc11af5c525581b771692c867c2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 28 Jun 2015 10:43:06 +0300 Subject: [PATCH 04/12] Added more error logging. --- server/modules/monitor/galeramon.c | 13 +++++++++++-- server/modules/monitor/mmmon.c | 26 ++++++++++++++++++++------ server/modules/monitor/mysql_mon.c | 22 +++++++++++++++++----- server/modules/monitor/ndbclustermon.c | 6 ++++-- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index e1ddc1217..f520b44aa 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -375,7 +375,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_state'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'wsrep_local_state'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } @@ -389,6 +390,13 @@ char *server_string; if (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); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW VARIABLES LIKE 'wsrep_sst_method'\". Expected 2 columns." + " MySQL Version: %s",version_str); + return; + } while ((row = mysql_fetch_row(result))) { if (strncmp(row[1], "xtrabackup", 10) == 0) @@ -409,7 +417,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_index'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'wsrep_local_index'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 970fd0d31..e70b3684a 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -370,7 +370,8 @@ char *server_string; if(mysql_field_count(database->con) != 1) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + skygw_log_write(LE,"Error: Unexpected result for 'SELECT @@server_id'. Expected 1 column." + " MySQL Version: %s",version_str); return; } @@ -403,8 +404,9 @@ char *server_string; if(mysql_field_count(database->con) < 42) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " - "returned less than the expected amount of rows."); + skygw_log_write(LE,"Error: \"SHOW ALL SLAVES STATUS\" " + "returned less than the expected amount of columns. Expected 42 columns" + " MySQL Version: %s",version_str); return; } @@ -450,8 +452,19 @@ char *server_string; if(mysql_field_count(database->con) < 40) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW SLAVE STATUS " - "returned less than the expected amount of rows."); + + if(server_version < 5*10000 + 5*100) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." + " MySQL Version: %s",version_str); + } + else + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + "returned less than the expected amount of columns. Expected 40 columns." + " MySQL Version: %s",version_str); + } return; } @@ -489,7 +502,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW GLOBAL VARIABLES LIKE 'read_only'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW GLOBAL VARIABLES LIKE 'read_only'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index 4c5f57ab6..d2fa7acbd 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -411,7 +411,8 @@ char *server_string; if(mysql_field_count(database->con) != 1) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + skygw_log_write(LE,"Error: Unexpected result for \"SELECT @@server_id\". Expected 1 columns." + " MySQL Version: %s",version_str); return; } while ((row = mysql_fetch_row(result))) @@ -443,8 +444,9 @@ char *server_string; if(mysql_field_count(database->con) < 42) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " - "returned less than the expected amount of rows."); + skygw_log_write(LE,"Error: \"SHOW ALL SLAVES STATUS\" " + "returned less than the expected amount of columns. Expected 42 columns." + " MySQL Version: %s",version_str); return; } @@ -489,8 +491,18 @@ char *server_string; if(mysql_field_count(database->con) < 40) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW SLAVE STATUS " - "returned less than the expected amount of rows."); + if(server_version < 5*10000 + 5*100) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." + " MySQL Version: %s",version_str); + } + else + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + "returned less than the expected amount of columns. Expected 40 columns." + " MySQL Version: %s",version_str); + } return; } diff --git a/server/modules/monitor/ndbclustermon.c b/server/modules/monitor/ndbclustermon.c index ed218d43e..a5d0b2455 100644 --- a/server/modules/monitor/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon.c @@ -326,7 +326,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } @@ -345,7 +346,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_cluster_node_id'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'Ndb_cluster_node_id'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } From 0062d9d2b783b752ee18a89fda6241a2c31c4ef9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 29 Jun 2015 10:24:16 +0300 Subject: [PATCH 05/12] Version errors for SHOW SLAVE STATUS now only print once. --- server/core/monitor.c | 1 + server/include/monitor.h | 1 + server/modules/monitor/mmmon.c | 11 ++++++++--- server/modules/monitor/mysql_mon.c | 11 ++++++++--- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/server/core/monitor.c b/server/core/monitor.c index 0f24e691d..f422b4f6b 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -192,6 +192,7 @@ monitorAddServer(MONITOR *mon, SERVER *server) db->con = NULL; db->next = NULL; db->mon_err_count = 0; + db->log_version_err = true; /** Server status is uninitialized */ db->mon_prev_status = -1; /* pending status is updated by get_replication_tree */ diff --git a/server/include/monitor.h b/server/include/monitor.h index 681f34bac..442efb27f 100644 --- a/server/include/monitor.h +++ b/server/include/monitor.h @@ -124,6 +124,7 @@ typedef enum typedef struct monitor_servers { SERVER *server; /**< The server being monitored */ MYSQL *con; /**< The MySQL connection */ + bool log_version_err; int mon_err_count; unsigned int mon_prev_status; unsigned int pending_status; /**< Pending Status flag bitmap */ diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index e70b3684a..40c2c11fb 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -455,9 +455,14 @@ char *server_string; if(server_version < 5*10000 + 5*100) { - skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " - " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." - " MySQL Version: %s",version_str); + if(database->log_version_err) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for versions less than 5.5 does not have master_server_id, " + "replication tree cannot be resolved for server %s." + " MySQL Version: %s",database->server->unique_name,version_str); + database->log_version_err = false; + } } else { diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index d2fa7acbd..2ba70a499 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -493,9 +493,14 @@ char *server_string; mysql_free_result(result); if(server_version < 5*10000 + 5*100) { - skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " - " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." - " MySQL Version: %s",version_str); + if(database->log_version_err) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for versions less than 5.5 does not have master_server_id, " + "replication tree cannot be resolved for server %s." + " MySQL Version: %s",database->server->unique_name,version_str); + database->log_version_err = false; + } } else { From 2c3c856ea1b781b0e61f21836bf4a4bd0434f336 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 29 Jun 2015 16:01:56 +0300 Subject: [PATCH 06/12] Added option for C99 builds. --- CMakeLists.txt | 8 +++++++- cmake/macros.cmake | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c28b2c657..62abd54a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,13 @@ if(PROFILE) set(FLAGS "${FLAGS} -pg " CACHE STRING "Compilation flags" FORCE) endif() -set(CMAKE_C_FLAGS "${FLAGS}") +if(USE_C99) + message(STATUS "Using C99 standard") + set(CMAKE_C_FLAGS "-std=c99 -D_GNU_SOURCE=1 ${FLAGS}") +else() + set(CMAKE_C_FLAGS "${FLAGS}") +endif() + set(CMAKE_C_FLAGS_DEBUG "${DEBUG_FLAGS} -DSS_DEBUG -DLOG_ASSERT") set(CMAKE_C_FLAGS_RELEASE "") set(CMAKE_C_FLAGS_RELWITHDEBINFO "-ggdb") diff --git a/cmake/macros.cmake b/cmake/macros.cmake index b78502cf9..d7921f1c4 100644 --- a/cmake/macros.cmake +++ b/cmake/macros.cmake @@ -20,6 +20,9 @@ endmacro() macro(set_variables) + # Use C99 + set(USE_C99 FALSE CACHE BOOL "Use C99 standard") + # hostname or IP address of MaxScale's host set(TEST_HOST "127.0.0.1" CACHE STRING "hostname or IP address of MaxScale's host") From 6f343ff57b7c599d0825c4094dfc6afb05ce140f Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 29 Jun 2015 19:17:12 +0300 Subject: [PATCH 07/12] Fix to MXS-227: https://mariadb.atlassian.net/browse/MXS-227 Fixed memory leak. --- server/modules/monitor/galeramon.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index f520b44aa..11f9c565a 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -280,7 +280,7 @@ monitorDatabase(MONITOR *mon, MONITOR_SERVERS *database) { GALERA_MONITOR* handle = (GALERA_MONITOR*)mon->handle; MYSQL_ROW row; -MYSQL_RES *result; +MYSQL_RES *result,*result2; int isjoined = 0; char *uname = mon->user; char *passwd = mon->password; @@ -388,11 +388,12 @@ char *server_string; /* 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) { if (mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0 - && (result = mysql_store_result(database->con)) != NULL) + && (result2 = mysql_store_result(database->con)) != NULL) { if(mysql_field_count(database->con) < 2) { mysql_free_result(result); + mysql_free_result(result2); skygw_log_write(LE,"Error: Unexpected result for \"SHOW VARIABLES LIKE 'wsrep_sst_method'\". Expected 2 columns." " MySQL Version: %s",version_str); return; @@ -402,6 +403,7 @@ char *server_string; if (strncmp(row[1], "xtrabackup", 10) == 0) isjoined = 1; } + mysql_free_result(result2); } } } From e350f19e6f07b0f3912d10a5937d8133cd2b94b9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 30 Jun 2015 14:58:36 +0300 Subject: [PATCH 08/12] Added NULL checks to readwritesplit. --- .../routing/readwritesplit/readwritesplit.c | 131 +++++++++++++----- 1 file changed, 99 insertions(+), 32 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 14f99c361..b5b4ddb36 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -226,7 +226,7 @@ static rses_property_t* mysql_sescmd_get_property( static rses_property_t* rses_property_init( rses_property_type_t prop_type); -static void rses_property_add( +static int rses_property_add( ROUTER_CLIENT_SES* rses, rses_property_t* prop); @@ -2943,6 +2943,11 @@ static void bref_clear_state( backend_ref_t* bref, bref_state_t state) { + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to bref_clear_state. (%s:%d)",__FILE__,__LINE__); + return; + } if (state != BREF_WAITING_RESULT) { bref->bref_state &= ~state; @@ -2972,6 +2977,11 @@ static void bref_set_state( backend_ref_t* bref, bref_state_t state) { + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to bref_set_state. (%s:%d)",__FILE__,__LINE__); + return; + } if (state != BREF_WAITING_RESULT) { bref->bref_state |= state; @@ -3535,7 +3545,8 @@ static rses_property_t* rses_property_init( prop = (rses_property_t*)calloc(1, sizeof(rses_property_t)); if (prop == NULL) { - goto return_prop; + skygw_log_write(LE,"Error: Malloc returned NULL. (%s:%d)",__FILE__,__LINE__); + return NULL; } prop->rses_prop_type = prop_type; #if defined(SS_DEBUG) @@ -3554,6 +3565,11 @@ return_prop: static void rses_property_done( rses_property_t* prop) { + if(prop == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to rses_property_done. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_RSES_PROP(prop); switch (prop->rses_prop_type) { @@ -3587,10 +3603,20 @@ static void rses_property_done( * * Router client session must be locked. */ -static void rses_property_add( +static int rses_property_add( ROUTER_CLIENT_SES* rses, rses_property_t* prop) { + if(rses == NULL) + { + skygw_log_write(LE,"Error: Router client session is NULL. (%s:%d)",__FILE__,__LINE__); + return -1; + } + if(prop == NULL) + { + skygw_log_write(LE,"Error: Router client session property is NULL. (%s:%d)",__FILE__,__LINE__); + return -1; + } rses_property_t* p; CHK_CLIENT_RSES(rses); @@ -3612,6 +3638,7 @@ static void rses_property_add( } p->rses_prop_next = prop; } + return 0; } /** @@ -3622,7 +3649,13 @@ static mysql_sescmd_t* rses_property_get_sescmd( rses_property_t* prop) { mysql_sescmd_t* sescmd; - + + if(prop == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to rses_property_get_sescmd. (%s:%d)",__FILE__,__LINE__); + return; + } + CHK_RSES_PROP(prop); ss_dassert(prop->rses_prop_rsession == NULL || SPINLOCK_IS_LOCKED(&prop->rses_prop_rsession->rses_lock)); @@ -3635,22 +3668,6 @@ static mysql_sescmd_t* rses_property_get_sescmd( } return sescmd; } - -/** -static void rses_begin_locked_property_action( - rses_property_t* prop) -{ - CHK_RSES_PROP(prop); - spinlock_acquire(&prop->rses_prop_lock); -} - -static void rses_end_locked_property_action( - rses_property_t* prop) -{ - CHK_RSES_PROP(prop); - spinlock_release(&prop->rses_prop_lock); -} -*/ /** * Create session command property. @@ -3683,6 +3700,11 @@ static mysql_sescmd_t* mysql_sescmd_init ( static void mysql_sescmd_done( mysql_sescmd_t* sescmd) { + if(sescmd == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to mysql_sescmd_done. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_RSES_PROP(sescmd->my_sescmd_prop); gwbuf_free(sescmd->my_sescmd_buf); memset(sescmd, 0, sizeof(mysql_sescmd_t)); @@ -3855,6 +3877,12 @@ static bool sescmd_cursor_is_active( sescmd_cursor_t* sescmd_cursor) { bool succp; + + if(sescmd_cursor == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_is_active. (%s:%d)",__FILE__,__LINE__); + return false; + } ss_dassert(SPINLOCK_IS_LOCKED(&sescmd_cursor->scmd_cur_rses->rses_lock)); succp = sescmd_cursor->scmd_cur_active; @@ -3880,6 +3908,11 @@ static GWBUF* sescmd_cursor_clone_querybuf( sescmd_cursor_t* scur) { GWBUF* buf; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_clone_querybuf. (%s:%d)",__FILE__,__LINE__); + return NULL; + } ss_dassert(scur->scmd_cur_cmd != NULL); buf = gwbuf_clone(scur->scmd_cur_cmd->my_sescmd_buf); @@ -3892,7 +3925,12 @@ static bool sescmd_cursor_history_empty( sescmd_cursor_t* scur) { bool succp; - + + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_history_empty. (%s:%d)",__FILE__,__LINE__); + return true; + } CHK_SESCMD_CUR(scur); if (scur->scmd_cur_rses->rses_properties[RSES_PROP_TYPE_SESCMD] == NULL) @@ -3912,6 +3950,11 @@ static void sescmd_cursor_reset( sescmd_cursor_t* scur) { ROUTER_CLIENT_SES* rses; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_reset. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_SESCMD_CUR(scur); CHK_CLIENT_RSES(scur->scmd_cur_rses); rses = scur->scmd_cur_rses; @@ -3928,6 +3971,11 @@ static bool execute_sescmd_history( { bool succp; sescmd_cursor_t* scur; + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_history. (%s:%d)",__FILE__,__LINE__); + return false; + } CHK_BACKEND_REF(bref); scur = &bref->bref_sescmd_cur; @@ -3964,6 +4012,11 @@ static bool execute_sescmd_in_backend( int rc = 0; sescmd_cursor_t* scur; + if(backend_ref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_in_backend. (%s:%d)",__FILE__,__LINE__); + return false; + } if (BREF_IS_CLOSED(backend_ref)) { succp = false; @@ -4084,6 +4137,12 @@ static bool sescmd_cursor_next( rses_property_t* prop_curr; rses_property_t* prop_next; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_next. (%s:%d)",__FILE__,__LINE__); + return false; + } + ss_dassert(scur != NULL); ss_dassert(*(scur->scmd_cur_ptr_property) != NULL); ss_dassert(SPINLOCK_IS_LOCKED( @@ -4410,11 +4469,21 @@ static bool route_session_write( * prevent it from being released before properties * are cleaned up as a part of router sessionclean-up. */ - prop = rses_property_init(RSES_PROP_TYPE_SESCMD); + if((prop = rses_property_init(RSES_PROP_TYPE_SESCMD)) == NULL) + { + skygw_log_write(LE,"Error: Router session property initialization failed"); + rses_end_locked_router_action(router_cli_ses); + return false; + } mysql_sescmd_init(prop, querybuf, packet_type, router_cli_ses); /** Add sescmd property to router client session */ - rses_property_add(router_cli_ses, prop); + if(rses_property_add(router_cli_ses, prop) != 0) + { + skygw_log_write(LE,"Error: Session property addition failed."); + rses_end_locked_router_action(router_cli_ses); + return false; + } for (i=0; irses_nbackends; i++) { @@ -4537,7 +4606,10 @@ static void rwsplit_process_router_options( int i; char* value; select_criteria_t c; - + + if(options == NULL) + return; + for (i = 0; options[i]; i++) { if ((value = strchr(options[i], '=')) == NULL) @@ -4625,7 +4697,7 @@ static void handleError ( SESSION* session; ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; - + CHK_DCB(backend_dcb); /** Reset error handle flag from a given DCB */ @@ -5088,8 +5160,6 @@ static int router_handle_state_switch( { backend_ref_t* bref; int rc = 1; - ROUTER_CLIENT_SES* rses; - SESSION* ses; SERVER* srv; CHK_DCB(dcb); @@ -5110,11 +5180,8 @@ static int router_handle_state_switch( srv->name, srv->port, STRSRVSTATUS(srv)))); - ses = dcb->session; - CHK_SESSION(ses); - - rses = (ROUTER_CLIENT_SES *)dcb->session->router_session; - CHK_CLIENT_RSES(rses); + CHK_SESSION(dcb->session); + CHK_CLIENT_RSES(dcb->session->router_session); switch (reason) { case DCB_REASON_NOT_RESPONDING: From a2e31d6846321c15a127f5cc9e372e9f1dc02759 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 30 Jun 2015 19:16:49 +0300 Subject: [PATCH 09/12] Changed the service resource hashing function into a proper string hashing function. --- server/core/dbusers.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 935644c18..6748a1437 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -1842,18 +1842,6 @@ char *mysql_format_user_entry(void *data) return mysql_user; } -/* - * The hash function we use for storing MySQL database names. - * - * @param key The key value - * @return The hash key - */ -int -resource_hash(char *key) -{ - return (*key + *(key + 1)); -} - /** * Remove the resources table * @@ -1877,7 +1865,7 @@ resource_alloc() { HASHTABLE *resources; - if ((resources = hashtable_alloc(10, resource_hash, strcmp)) == NULL) + if ((resources = hashtable_alloc(10, simple_str_hash, strcmp)) == NULL) { return NULL; } From 6c61dabfab706b654637e166aecc49966194fd50 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 1 Jul 2015 05:15:08 +0300 Subject: [PATCH 10/12] Fix to a Coverity defect. --- server/modules/routing/readwritesplit/readwritesplit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b5b4ddb36..c4008e65e 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3653,7 +3653,7 @@ static mysql_sescmd_t* rses_property_get_sescmd( if(prop == NULL) { skygw_log_write(LE,"Error: NULL parameter passed to rses_property_get_sescmd. (%s:%d)",__FILE__,__LINE__); - return; + return NULL; } CHK_RSES_PROP(prop); From cc24777a902f85966bd23ce73748a66f73ad02a2 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Wed, 1 Jul 2015 09:46:01 +0100 Subject: [PATCH 11/12] Correct mkdir logic for default log directory. --- server/core/gateway.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index c474083ee..8f486067c 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -1531,17 +1531,13 @@ int main(int argc, char **argv) /** Use default log directory /var/log/maxscale/ */ if(logdir == NULL) { - - if(access(default_logdir,F_OK) != 0) - { - if(mkdir(logdir,0555) != 0) - { - fprintf(stderr, - "Error: Cannot create log directory: %s\n", - default_logdir); - goto return_main; - } - } + if(mkdir(default_logdir,0777) != 0 && errno != EEXIST) + { + fprintf(stderr, + "Error: Cannot create log directory: %s\n", + default_logdir); + goto return_main; + } logdir = strdup(default_logdir); } @@ -1598,7 +1594,7 @@ int main(int argc, char **argv) /** * Set a data directory for the mysqld library, we use * a unique directory name to avoid clauses if multiple - * instances of the gateway are beign run on the same + * instances of the gateway are being run on the same * machine. */ From 22f8e61321a9c5bad6c7c02aca8c1e1687666113 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 1 Jul 2015 16:46:32 +0300 Subject: [PATCH 12/12] Fixed build failure. --- server/modules/routing/readwritesplit/readwritesplit.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index c4008e65e..b801d9aec 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -5161,7 +5161,8 @@ static int router_handle_state_switch( backend_ref_t* bref; int rc = 1; SERVER* srv; - + ROUTER_CLIENT_SES* rses; + SESSION* ses; CHK_DCB(dcb); bref = (backend_ref_t *)data; CHK_BACKEND_REF(bref); @@ -5180,8 +5181,10 @@ static int router_handle_state_switch( srv->name, srv->port, STRSRVSTATUS(srv)))); - CHK_SESSION(dcb->session); - CHK_CLIENT_RSES(dcb->session->router_session); + ses = dcb->session; + CHK_SESSION(ses); + rses = (ROUTER_CLIENT_SES *)dcb->session->router_session; + CHK_CLIENT_RSES(rses); switch (reason) { case DCB_REASON_NOT_RESPONDING: