From de1e1f4e28f486ea876754a248104b0f4eb4bdea Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 28 Sep 2015 10:08:50 +0300 Subject: [PATCH 01/10] In test-programs the ss...assert functions assert always. Now handled by defining the relevant defines. Should be fixed by replacing the use of ss_info_assert with test macros that always assert. Task for fixing this properly: https://mariadb.atlassian.net/browse/MXS-382 --- server/core/test/testbuffer.c | 7 +++++++ server/core/test/testservice.c | 7 +++++++ server/core/test/testusers.c | 7 +++++++ 3 files changed, 21 insertions(+) diff --git a/server/core/test/testbuffer.c b/server/core/test/testbuffer.c index 73d71dc27..179300be3 100644 --- a/server/core/test/testbuffer.c +++ b/server/core/test/testbuffer.c @@ -27,6 +27,13 @@ * @endverbatim */ +// To ensure that ss_info_assert asserts also when builing in non-debug mode. +#if !defined(SS_DEBUG) +#define SS_DEBUG +#endif +#if defined(NDEBUG) +#undef NDEBUG +#endif #include #include #include diff --git a/server/core/test/testservice.c b/server/core/test/testservice.c index f45fd2afc..57542a64f 100644 --- a/server/core/test/testservice.c +++ b/server/core/test/testservice.c @@ -27,6 +27,13 @@ * @endverbatim */ +// To ensure that ss_info_assert asserts also when builing in non-debug mode. +#if !defined(SS_DEBUG) +#define SS_DEBUG +#endif +#if defined(NDEBUG) +#undef NDEBUG +#endif #include #include #include diff --git a/server/core/test/testusers.c b/server/core/test/testusers.c index d947075a0..4385d74eb 100644 --- a/server/core/test/testusers.c +++ b/server/core/test/testusers.c @@ -27,6 +27,13 @@ * @endverbatim */ +// To ensure that ss_info_assert asserts also when builing in non-debug mode. +#if !defined(SS_DEBUG) +#define SS_DEBUG +#endif +#if defined(NDEBUG) +#undef NDEBUG +#endif #include #include #include From fcb13760fb3e13a4a74d9d1dfa804a9a82bcbac3 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 28 Sep 2015 10:27:43 +0300 Subject: [PATCH 02/10] Warnings treated as errors. With all current warnings removed, warnings can now be treated as errors. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9bae1af40..6780205fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -96,7 +96,7 @@ configure_file(${CMAKE_SOURCE_DIR}/etc/postinst.in ${CMAKE_BINARY_DIR}/postinst configure_file(${CMAKE_SOURCE_DIR}/etc/postrm.in ${CMAKE_BINARY_DIR}/postrm @ONLY) configure_file(${CMAKE_SOURCE_DIR}/server/test/maxscale_test.cnf ${CMAKE_BINARY_DIR}/maxscale.cnf @ONLY) -set(FLAGS "-Wall -Wno-unused-variable -Wno-unused-function -fPIC" CACHE STRING "Compilation flags") +set(FLAGS "-Wall -Wno-unused-variable -Wno-unused-function -Werror -fPIC" CACHE STRING "Compilation flags") set(DEBUG_FLAGS "-ggdb -pthread -pipe -Wformat -fstack-protector --param=ssp-buffer-size=4" CACHE STRING "Debug compilation flags") if(CMAKE_VERSION VERSION_GREATER 2.6) From 6f3ec723b12338894a4519f39b0502d5544efbe1 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 29 Sep 2015 10:23:04 +0300 Subject: [PATCH 03/10] Cleaned up Limitations --- Documentation/About/Limitations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/About/Limitations.md b/Documentation/About/Limitations.md index 4677640a9..ec0d7f357 100644 --- a/Documentation/About/Limitations.md +++ b/Documentation/About/Limitations.md @@ -10,8 +10,6 @@ This section describes the limitations that are common to all configuration of p Compression is not included in MySQL server handshake -## Limitations with MySQL Master/Slave Replication monitoring - ## Limitations with Galera Cluster Monitoring Master selection is based only on MIN(wsrep_local_index), no other server parameter. @@ -35,7 +33,9 @@ In master-slave replication cluster also read-only queries are routed to master * statement includes a stored procedure, or an UDF call ### Limitations in client session handling + Some of the queries that client sends are routed to all backends instead of sending them just to one of server. These queries include `USE ` and `SET autocommit=0` among many others. Readwritesplit sends a copy of these queries to each backend server and forwards the master's reply to the client. Below is a list of MySQL commands which are classified as session commands : + ``` COM_INIT_DB (USE creates this) @@ -90,4 +90,4 @@ Most imaginable reasons are related to replication lag but it could be possible ## Authentication Related Limitations -MySQL old passwords are not supported +MySQL old style passwords are not supported. MySQL versions 4.1 and newer use a new authentication protocol which does not support pre-4.1 style passwords. From db0e2e881f3477e62950bb5dcc13783b4e680ab5 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 29 Sep 2015 10:37:56 +0300 Subject: [PATCH 04/10] Double free prevented. routeQuery calls route_single_stmt, which requires the GWBUF to be contiguous. Earlier it was made contiguous (if needed) in route_single_stmt. However, since the process of making a GWBUF contiguous causes the original buffer to be freed, this would lead to a double free later in routeQuery that frees the passed buffer. This is prevented now by making the buffer contiguous before calling route_single_stmt. --- .../modules/routing/readwritesplit/readwritesplit.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 9a322d1f3..60c724958 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -2022,6 +2022,9 @@ static int routeQuery( } else { + /** route_single_stmt expects the buffer to be contiguous. */ + querybuf = gwbuf_make_contiguous(querybuf); + succp = route_single_stmt(inst, router_cli_ses, querybuf); } } @@ -2053,6 +2056,9 @@ static int routeQuery( } else { + /** route_single_stmt expects the buffer to be contiguous. */ + querybuf = gwbuf_make_contiguous(querybuf); + succp = route_single_stmt(inst, router_cli_ses, querybuf); } @@ -2108,6 +2114,7 @@ static bool route_single_stmt( int rlag_max = MAX_RLAG_UNDEFINED; backend_type_t btype; /*< target backend type */ + ss_dassert(querybuf->next == NULL); // The buffer must be contiguous. ss_dassert(!GWBUF_IS_TYPE_UNDEFINED(querybuf)); /** @@ -2130,12 +2137,6 @@ static bool route_single_stmt( succp = false; goto retblock; } - - /** If buffer is not contiguous, make it such */ - if (querybuf->next != NULL) - { - querybuf = gwbuf_make_contiguous(querybuf); - } packet = GWBUF_DATA(querybuf); packet_len = gw_mysql_get_byte3(packet); From f021d428205b01a17adceb98b29b8f499404c2fe Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 22 Sep 2015 14:17:33 +0300 Subject: [PATCH 05/10] Cleaned up the timerange string processing in dbfwfilter. --- server/modules/filter/dbfwfilter.c | 123 +++++++++++++++++------------ 1 file changed, 74 insertions(+), 49 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index e3850f841..b64ec58da 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -473,7 +473,10 @@ bool check_time(char* str) #define IS_RVRS_TIME(tr) (mktime(&tr->end) < mktime(&tr->start)) /** - * Parses a null-terminated string into two tm_t structs that mark a timerange + * Parses a null-terminated string into a timerange defined by two ISO-8601 compliant + * times separated by a single dash. The times are interpreted at one second precision + * and follow the extended format by separating the hours, minutes and seconds with + * semicolons. * @param str String to parse * @param instance FW_FILTER instance * @return If successful returns a pointer to the new TIMERANGE instance. If errors occurred or @@ -482,63 +485,85 @@ bool check_time(char* str) TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) { - TIMERANGE* tr = NULL; - int intbuffer[3]; - int* idest = intbuffer; - char strbuffer[3]; - char *ptr,*sdest; - struct tm* tmptr; - assert(str != NULL && instance != NULL); - - tr = (TIMERANGE*)calloc(1,sizeof(TIMERANGE)); + + char strbuf[strlen(str) + 1]; + char *tok, *saved, *numend; + struct tm start, end; + + memset(&start,0,sizeof(struct tm)); + memset(&end,0,sizeof(struct tm)); + strncpy(strbuf, str, sizeof(strbuf)); + + /** Process the start of the timerange */ + if((tok = strtok_r(strbuf, "-: ", &saved)) == NULL) + return NULL; + + start.tm_hour = strtol(tok, &numend, 10); + + if(*numend != '\0') + return NULL; + + if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) + return NULL; + + start.tm_min = strtol(tok, &numend, 10); + + if(*numend != '\0') + return NULL; + + if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) + return NULL; + + start.tm_sec = strtol(tok, &numend, 10); + + if(*numend != '\0') + return NULL; + + /** Process the end of the timerange */ + + if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) + return NULL; + + start.tm_hour = strtol(tok, &numend, 10); + + if(*numend != '\0') + return NULL; + + if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) + return NULL; + + start.tm_min = strtol(tok, &numend, 10); + + if(*numend != '\0') + return NULL; + + if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) + return NULL; + + start.tm_sec = strtol(tok, &numend, 10); + + if(*numend != '\0') + return NULL; + + CHK_TIMES((&start)); + CHK_TIMES((&end)); + + /** The time string was valid */ + + TIMERANGE* tr = (TIMERANGE*)malloc(sizeof(TIMERANGE)); if(tr == NULL){ skygw_log_write(LOGFILE_ERROR, "dbfwfilter: malloc returned NULL."); return NULL; } - memset(&tr->start,0,sizeof(struct tm)); - memset(&tr->end,0,sizeof(struct tm)); - ptr = str; - sdest = strbuffer; - tmptr = &tr->start; - - while(ptr - str < 19){ - if(isdigit(*ptr)){ - *sdest = *ptr; - }else if(*ptr == ':' ||*ptr == '-' || *ptr == '\0' || *ptr == ' '){ - *sdest = '\0'; - *idest++ = atoi(strbuffer); - sdest = strbuffer; - - if(*ptr == '-' || *ptr == '\0'){ - - tmptr->tm_hour = intbuffer[0]; - tmptr->tm_min = intbuffer[1]; - tmptr->tm_sec = intbuffer[2]; - - CHK_TIMES(tmptr); - - if(*ptr == '\0' || *ptr == ' '){ - return tr; - } - - idest = intbuffer; - tmptr = &tr->end; - } - ptr++; - continue; - } - ptr++; - sdest++; - } - - free(tr); - return NULL; + memcpy(&tr->start, &start, sizeof(start)); + memcpy(&tr->end, &end, sizeof(end)); + tr->next = NULL; + return tr; } - /** * Splits the reversed timerange into two. *@param tr A reversed timerange From abab715a21164c39717f71778cd39751a473f580 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 22 Sep 2015 20:57:37 +0300 Subject: [PATCH 06/10] Simplified the code in parse_time. --- server/modules/filter/dbfwfilter.c | 94 +++++++++--------------------- 1 file changed, 26 insertions(+), 68 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index b64ec58da..daae2e848 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -472,6 +472,7 @@ bool check_time(char* str) #endif #define IS_RVRS_TIME(tr) (mktime(&tr->end) < mktime(&tr->start)) + /** * Parses a null-terminated string into a timerange defined by two ISO-8601 compliant * times separated by a single dash. The times are interpreted at one second precision @@ -485,83 +486,40 @@ bool check_time(char* str) TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) { - assert(str != NULL && instance != NULL); + assert(str != NULL && instance != NULL); char strbuf[strlen(str) + 1]; - char *tok, *saved, *numend; - struct tm start, end; + char *separator; + struct tm start, end; + TIMERANGE* tr = NULL; - memset(&start,0,sizeof(struct tm)); - memset(&end,0,sizeof(struct tm)); strncpy(strbuf, str, sizeof(strbuf)); - /** Process the start of the timerange */ - if((tok = strtok_r(strbuf, "-: ", &saved)) == NULL) - return NULL; + if ((separator = strchr(strbuf, '-'))) + { + *separator++ = '\0'; + if (strptime(strbuf, "%H:%M:%S", &start) && + strptime(separator, "%H:%M:%S", &end)) + { + CHK_TIMES((&start)); + CHK_TIMES((&end)); - start.tm_hour = strtol(tok, &numend, 10); + /** The time string was valid */ - if(*numend != '\0') - return NULL; + tr = (TIMERANGE*) malloc(sizeof(TIMERANGE)); - if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) - return NULL; + if (tr == NULL) + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: malloc returned NULL."); + return NULL; + } - start.tm_min = strtol(tok, &numend, 10); - - if(*numend != '\0') - return NULL; - - if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) - return NULL; - - start.tm_sec = strtol(tok, &numend, 10); - - if(*numend != '\0') - return NULL; - - /** Process the end of the timerange */ - - if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) - return NULL; - - start.tm_hour = strtol(tok, &numend, 10); - - if(*numend != '\0') - return NULL; - - if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) - return NULL; - - start.tm_min = strtol(tok, &numend, 10); - - if(*numend != '\0') - return NULL; - - if((tok = strtok_r(NULL, "-: ", &saved)) == NULL) - return NULL; - - start.tm_sec = strtol(tok, &numend, 10); - - if(*numend != '\0') - return NULL; - - CHK_TIMES((&start)); - CHK_TIMES((&end)); - - /** The time string was valid */ - - TIMERANGE* tr = (TIMERANGE*)malloc(sizeof(TIMERANGE)); - - if(tr == NULL){ - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: malloc returned NULL."); - return NULL; - } - - memcpy(&tr->start, &start, sizeof(start)); - memcpy(&tr->end, &end, sizeof(end)); - tr->next = NULL; - return tr; + memcpy(&tr->start, &start, sizeof(start)); + memcpy(&tr->end, &end, sizeof(end)); + tr->next = NULL; + } + } + return tr; } /** From 447c3aa6c1d713bdbc62440cac95a5d438580fd4 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 23 Sep 2015 08:40:59 +0300 Subject: [PATCH 07/10] More code cleaning. --- server/modules/filter/dbfwfilter.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index daae2e848..8f64139c8 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -483,17 +483,16 @@ bool check_time(char* str) * @return If successful returns a pointer to the new TIMERANGE instance. If errors occurred or * the timerange was invalid, a NULL pointer is returned. */ -TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) +static TIMERANGE* parse_time(const char* str) { - - assert(str != NULL && instance != NULL); + assert(str != NULL); char strbuf[strlen(str) + 1]; char *separator; struct tm start, end; TIMERANGE* tr = NULL; - strncpy(strbuf, str, sizeof(strbuf)); + strcpy(strbuf, str); if ((separator = strchr(strbuf, '-'))) { @@ -501,22 +500,22 @@ TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) if (strptime(strbuf, "%H:%M:%S", &start) && strptime(separator, "%H:%M:%S", &end)) { + /** The time string was valid */ CHK_TIMES((&start)); CHK_TIMES((&end)); - /** The time string was valid */ - tr = (TIMERANGE*) malloc(sizeof(TIMERANGE)); - if (tr == NULL) + if (tr) + { + tr->start = start; + tr->end = end; + tr->next = NULL; + } + else { skygw_log_write(LOGFILE_ERROR, "dbfwfilter: malloc returned NULL."); - return NULL; } - - memcpy(&tr->start, &start, sizeof(start)); - memcpy(&tr->end, &end, sizeof(end)); - tr->next = NULL; } } return tr; @@ -1003,7 +1002,7 @@ bool parse_rule(char* rule, FW_INSTANCE* instance) goto retblock; } - TIMERANGE *tmp = parse_time(tok,instance); + TIMERANGE *tmp = parse_time(tok); if(tmp == NULL) { From 96f76a1f2eba4d1d368c4ba430adc3095113fd88 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 25 Sep 2015 21:23:09 +0300 Subject: [PATCH 08/10] Changed the way max_sescmd_history works and combined disable_sescmd_history and disable_slave_recovery. Before these changes when max_sescmd_history was used the session was closed when the limit was exceeded. With this change, when the limit is exceeded the recovery of slaves and the session command history are both disabled. This will allow the sessions to continue while still keeping the old functionality of limited salve replacement. The disable_sescmd_history and disable_slave_recovery parameters were combined so that disabling the session command history will also disable slave recovery. This way no harm can be done with disable_sescmd_history. --- Documentation/Routers/ReadWriteSplit.md | 14 +++---- server/modules/include/readwritesplit.h | 22 +++++----- .../routing/readwritesplit/readwritesplit.c | 40 ++++++++----------- 3 files changed, 31 insertions(+), 45 deletions(-) diff --git a/Documentation/Routers/ReadWriteSplit.md b/Documentation/Routers/ReadWriteSplit.md index f75a5873b..ec89db65f 100644 --- a/Documentation/Routers/ReadWriteSplit.md +++ b/Documentation/Routers/ReadWriteSplit.md @@ -73,9 +73,12 @@ When value all is used, queries reading session variables can be routed to any a In above-mentioned case the user-defined variable would only be updated in the master where query would be routed due to `INSERT` statement. -**`max_sescmd_history`** sets a limit on how many session commands each session can execute before the connection is closed. The default is an unlimited number of session commands. +**`max_sescmd_history`** sets a limit on how many session commands each session can execute before the session command history is disabled. The default is an unlimited number of session commands. - max_sescmd_history=1500 +``` +# Set a limit on the session command history +max_sescmd_history=1500 +``` When a limitation is set, it effectively creates a cap on the session's memory consumption. This might be useful if connection pooling is used and the sessions use large amounts of session commands. @@ -86,13 +89,6 @@ When a limitation is set, it effectively creates a cap on the session's memory c disable_sescmd_history=true ``` -**`disable_slave_recovery`** disables the recovery and replacement of slave servers. If this option is enabled and a connection to a slave server in use is lost, no replacement slave will be taken. This allows the safe use of session state modifying statements when the session command history is disabled. This is mostly intended to be used with the `disable_sescmd_history` option enabled. - -``` -# Disable the session command history -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. ``` diff --git a/server/modules/include/readwritesplit.h b/server/modules/include/readwritesplit.h index c78cde5b4..9f9866e5d 100644 --- a/server/modules/include/readwritesplit.h +++ b/server/modules/include/readwritesplit.h @@ -242,19 +242,17 @@ typedef struct backend_ref_st { #endif } backend_ref_t; - -typedef struct rwsplit_config_st { - int rw_max_slave_conn_percent; - int rw_max_slave_conn_count; - select_criteria_t rw_slave_select_criteria; - int rw_max_slave_replication_lag; - target_t rw_use_sql_variables_in; - int rw_max_sescmd_history_size; - bool disable_sescmd_hist; - bool disable_slave_recovery; - bool master_reads; /*< Use master for reads */ +typedef struct rwsplit_config_st +{ + int rw_max_slave_conn_percent; + int rw_max_slave_conn_count; + select_criteria_t rw_slave_select_criteria; + int rw_max_slave_replication_lag; + target_t rw_use_sql_variables_in; + int rw_max_sescmd_history_size; + bool rw_disable_sescmd_hist; + bool rw_master_reads; /*< Use master for reads */ } rwsplit_config_t; - #if defined(PREP_STMT_CACHING) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 60c724958..15ad5ca42 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -702,7 +702,7 @@ createInstance(SERVICE *service, char **options) } /** These options cancel each other out */ - if(router->rwsplit_config.disable_sescmd_hist && router->rwsplit_config.rw_max_sescmd_history_size > 0) + if(router->rwsplit_config.rw_disable_sescmd_hist && router->rwsplit_config.rw_max_sescmd_history_size > 0) { router->rwsplit_config.rw_max_sescmd_history_size = 0; } @@ -1246,7 +1246,7 @@ static bool get_dcb( (max_rlag == MAX_RLAG_UNDEFINED || (b->backend_server->rlag != MAX_RLAG_NOT_AVAILABLE && b->backend_server->rlag <= max_rlag)) && - !rses->rses_config.master_reads) + !rses->rses_config.rw_master_reads) { /** found slave */ candidate_bref = &backend_ref[i]; @@ -2897,7 +2897,7 @@ static void clientReply ( bool rconn = false; writebuf = sescmd_cursor_process_replies(writebuf, bref, &rconn); - if(rconn && !router_inst->rwsplit_config.disable_slave_recovery) + if(rconn && !router_inst->rwsplit_config.rw_disable_sescmd_hist) { select_connect_backend_servers(&router_cli_ses->rses_master_ref, router_cli_ses->rses_backend_ref, @@ -4541,21 +4541,17 @@ static bool route_session_write( goto return_succp; } - if(router_cli_ses->rses_config.rw_max_sescmd_history_size > 0 && - router_cli_ses->rses_nsescmd >= router_cli_ses->rses_config.rw_max_sescmd_history_size) - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Router session exceeded session command history limit. " - "Closing router session. <"))); - gwbuf_free(querybuf); - rses_end_locked_router_action(router_cli_ses); - router_cli_ses->client_dcb->func.hangup(router_cli_ses->client_dcb); - - goto return_succp; - } + if (router_cli_ses->rses_config.rw_max_sescmd_history_size > 0 && + router_cli_ses->rses_nsescmd >= router_cli_ses->rses_config.rw_max_sescmd_history_size) + { + skygw_log_write(LE, "Warning: Router session exceeded session command history limit. " + "Slave recovery is disabled and only slave servers with consistent session state are used " + "for the duration of the session."); + router_cli_ses->rses_config.rw_disable_sescmd_hist = true; + router_cli_ses->rses_config.rw_max_sescmd_history_size = 0; + } - if(router_cli_ses->rses_config.disable_sescmd_hist) + if(router_cli_ses->rses_config.rw_disable_sescmd_hist) { rses_property_t *prop, *tmp; backend_ref_t* bref; @@ -4784,15 +4780,11 @@ static void rwsplit_process_router_options( } else if(strcmp(options[i],"disable_sescmd_history") == 0) { - router->rwsplit_config.disable_sescmd_hist = config_truth_value(value); - } - else if(strcmp(options[i],"disable_slave_recovery") == 0) - { - router->rwsplit_config.disable_slave_recovery = config_truth_value(value); + router->rwsplit_config.rw_disable_sescmd_hist = config_truth_value(value); } else if(strcmp(options[i],"master_accept_reads") == 0) { - router->rwsplit_config.master_reads = config_truth_value(value); + router->rwsplit_config.rw_master_reads = config_truth_value(value); } } } /*< for */ @@ -5039,7 +5031,7 @@ static bool handle_error_new_connection( * Try to get replacement slave or at least the minimum * number of slave connections for router session. */ - if(inst->rwsplit_config.disable_slave_recovery) + if(inst->rwsplit_config.rw_disable_sescmd_hist) { succp = have_enough_servers(&myrses,1,router_nservers,inst) ? true : false; } From d679bf1cd8846ddae821fede23a81d5dc3a36926 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 29 Sep 2015 14:52:26 +0300 Subject: [PATCH 09/10] Fix to MXS-389: https://mariadb.atlassian.net/browse/MXS-389 Utility tools now use static log manager. --- server/core/CMakeLists.txt | 8 ++++---- server/modules/routing/binlog/CMakeLists.txt | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/server/core/CMakeLists.txt b/server/core/CMakeLists.txt index a2e9208e8..f9fb47657 100644 --- a/server/core/CMakeLists.txt +++ b/server/core/CMakeLists.txt @@ -23,12 +23,12 @@ endif() target_link_libraries(maxscale ${EMBEDDED_LIB} ${PCRE_LINK_FLAGS} ${CURL_LIBRARIES} log_manager utils ssl aio pthread crypt dl crypto inih z rt m stdc++) install(TARGETS maxscale DESTINATION ${MAXSCALE_BINDIR}) -add_executable(maxkeys maxkeys.c secrets.c utils.c gwdirs.c) -target_link_libraries(maxkeys log_manager utils pthread crypt crypto) +add_executable(maxkeys maxkeys.c secrets.c utils.c gwdirs.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) +target_link_libraries(maxkeys utils pthread crypt crypto) install(TARGETS maxkeys DESTINATION ${MAXSCALE_BINDIR}) -add_executable(maxpasswd maxpasswd.c secrets.c utils.c gwdirs.c) -target_link_libraries(maxpasswd log_manager utils pthread crypt crypto) +add_executable(maxpasswd maxpasswd.c secrets.c utils.c gwdirs.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) +target_link_libraries(maxpasswd utils pthread crypt crypto) install(TARGETS maxpasswd DESTINATION ${MAXSCALE_BINDIR}) if(BUILD_TESTS) diff --git a/server/modules/routing/binlog/CMakeLists.txt b/server/modules/routing/binlog/CMakeLists.txt index bdf61aea1..e114ff9ea 100644 --- a/server/modules/routing/binlog/CMakeLists.txt +++ b/server/modules/routing/binlog/CMakeLists.txt @@ -3,8 +3,21 @@ set_target_properties(binlogrouter PROPERTIES INSTALL_RPATH ${CMAKE_INSTALL_RPAT target_link_libraries(binlogrouter ssl pthread log_manager) install(TARGETS binlogrouter DESTINATION ${MAXSCALE_LIBDIR}) -add_executable(maxbinlogcheck maxbinlogcheck.c blr_file.c blr_cache.c blr_master.c blr_slave.c blr.c ${CMAKE_SOURCE_DIR}/server/core/service.c ${CMAKE_SOURCE_DIR}/server/core/spinlock.c ${CMAKE_SOURCE_DIR}/server/core/buffer.c ${CMAKE_SOURCE_DIR}/server/core/atomic.c ${CMAKE_SOURCE_DIR}/server/core/hint.c ${CMAKE_SOURCE_DIR}/server/core/gwdirs.c ${CMAKE_SOURCE_DIR}/server/core/server.c ${CMAKE_SOURCE_DIR}/server/core/dcb.c ${CMAKE_SOURCE_DIR}/server/core/users.c ${CMAKE_SOURCE_DIR}/server/core/dbusers.c ${CMAKE_SOURCE_DIR}/server/core/utils.c ${CMAKE_SOURCE_DIR}/server/core/hashtable.c ${CMAKE_SOURCE_DIR}/server/core/poll.c ${CMAKE_SOURCE_DIR}/server/core/gwbitmask.c ${CMAKE_SOURCE_DIR}/server/core/config.c ${CMAKE_SOURCE_DIR}/server/core/session.c ${CMAKE_SOURCE_DIR}/server/core/housekeeper.c ${CMAKE_SOURCE_DIR}/server/core/filter.c ${CMAKE_SOURCE_DIR}/server/core/resultset.c ${CMAKE_SOURCE_DIR}/server/core/load_utils.c ${CMAKE_SOURCE_DIR}/server/core/monitor.c ${CMAKE_SOURCE_DIR}/server/core/gw_utils.c ${CMAKE_SOURCE_DIR}/server/core/thread.c ${CMAKE_SOURCE_DIR}/server/core/secrets.c) +add_executable(maxbinlogcheck maxbinlogcheck.c blr_file.c blr_cache.c blr_master.c blr_slave.c blr.c + ${CMAKE_SOURCE_DIR}/server/core/service.c ${CMAKE_SOURCE_DIR}/server/core/spinlock.c + ${CMAKE_SOURCE_DIR}/server/core/buffer.c ${CMAKE_SOURCE_DIR}/server/core/atomic.c + ${CMAKE_SOURCE_DIR}/server/core/hint.c ${CMAKE_SOURCE_DIR}/server/core/gwdirs.c + ${CMAKE_SOURCE_DIR}/server/core/server.c ${CMAKE_SOURCE_DIR}/server/core/dcb.c + ${CMAKE_SOURCE_DIR}/server/core/users.c ${CMAKE_SOURCE_DIR}/server/core/dbusers.c + ${CMAKE_SOURCE_DIR}/server/core/utils.c ${CMAKE_SOURCE_DIR}/server/core/hashtable.c + ${CMAKE_SOURCE_DIR}/server/core/poll.c ${CMAKE_SOURCE_DIR}/server/core/gwbitmask.c + ${CMAKE_SOURCE_DIR}/server/core/config.c ${CMAKE_SOURCE_DIR}/server/core/session.c + ${CMAKE_SOURCE_DIR}/server/core/housekeeper.c ${CMAKE_SOURCE_DIR}/server/core/filter.c + ${CMAKE_SOURCE_DIR}/server/core/resultset.c ${CMAKE_SOURCE_DIR}/server/core/load_utils.c + ${CMAKE_SOURCE_DIR}/server/core/monitor.c ${CMAKE_SOURCE_DIR}/server/core/gw_utils.c + ${CMAKE_SOURCE_DIR}/server/core/thread.c ${CMAKE_SOURCE_DIR}/server/core/secrets.c + ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) -target_link_libraries(maxbinlogcheck utils ssl pthread log_manager ${EMBEDDED_LIB} ${PCRE_LINK_FLAGS} aio rt crypt dl crypto inih z m stdc++ ${CURL_LIBRARIES}) +target_link_libraries(maxbinlogcheck utils ssl pthread ${EMBEDDED_LIB} ${PCRE_LINK_FLAGS} aio rt crypt dl crypto inih z m stdc++ ${CURL_LIBRARIES}) install(TARGETS maxbinlogcheck DESTINATION bin) From 638c2250c94ae5de61be03a75c8476543b2858ba Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 29 Sep 2015 16:36:12 +0300 Subject: [PATCH 10/10] Fixed internal test suite. --- server/core/adminusers.c | 7 ++- server/core/config.c | 1 + server/core/test/testadminusers.c | 2 +- server/core/test/testdcb.c | 2 +- server/core/test/testfeedback.c | 7 ++- server/core/test/testmodutil.c | 5 +- server/core/test/testpoll.c | 2 +- server/core/test/testservice.c | 49 ++++--------------- .../test/test_hints/rwsplit_hints.sh | 2 +- 9 files changed, 29 insertions(+), 48 deletions(-) diff --git a/server/core/adminusers.c b/server/core/adminusers.c index 8af593506..81df4bf4f 100644 --- a/server/core/adminusers.c +++ b/server/core/adminusers.c @@ -29,7 +29,7 @@ #include #include #include - +#include /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; extern size_t log_ses_count[]; @@ -154,6 +154,11 @@ FILE *fp; char fname[1024], *home, *cpasswd; initialise(); + + if(access(get_datadir(), F_OK) != 0) + if(mkdir(get_datadir(), S_IRWXU) != 0 && errno != EEXIST) + return ADMIN_ERR_PWDFILEOPEN; + snprintf(fname,1023, "%s/passwd", get_datadir()); fname[1023] = '\0'; if (users == NULL) diff --git a/server/core/config.c b/server/core/config.c index 1a963d562..463122138 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -2334,6 +2334,7 @@ static char *InternalRouters[] = { "cli", "maxinfo", "binlogrouter", + "testroute", NULL }; diff --git a/server/core/test/testadminusers.c b/server/core/test/testadminusers.c index f9c34e500..cfa9be6ba 100644 --- a/server/core/test/testadminusers.c +++ b/server/core/test/testadminusers.c @@ -270,7 +270,7 @@ char *home, buf[1024]; /* Unlink any existing password file before running this test */ - sprintf(buf, "%s/passwd", default_cachedir); + sprintf(buf, "%s/passwd", get_datadir()); if(!is_valid_posix_path(buf)) exit(1); if (strcmp(buf, "/etc/passwd") != 0) diff --git a/server/core/test/testdcb.c b/server/core/test/testdcb.c index 13462fc87..9668d8a23 100644 --- a/server/core/test/testdcb.c +++ b/server/core/test/testdcb.c @@ -59,7 +59,7 @@ int buflen; printAllDCBs(); ss_info_dassert(true, "Something is true"); ss_dfprintf(stderr, "\t..done\n"); - dcb_free(dcb); + dcb_close(dcb); ss_dfprintf(stderr, "Freed original dcb"); ss_info_dassert(!dcb_isvalid(dcb), "Freed DCB must not be valid"); ss_dfprintf(stderr, "\t..done\nMake clone DCB a zombie"); diff --git a/server/core/test/testfeedback.c b/server/core/test/testfeedback.c index 2947b0975..c1d53b9cc 100644 --- a/server/core/test/testfeedback.c +++ b/server/core/test/testfeedback.c @@ -41,6 +41,7 @@ #include #include #include +#include static char* server_options[] = { "MariaDB Corporation MaxScale", @@ -65,6 +66,7 @@ static char* server_groups[] = { }; int config_load(char *); +void config_enable_feedback_task(void); int module_create_feedback_report(GWBUF **buffer, MODULES *modules, FEEDBACK_CONF *cfg); int do_http_post(GWBUF *buffer, void *cfg); @@ -78,7 +80,8 @@ int main(int argc, char** argv) hkinit(); - cnf = strdup("/etc/MaxScale.cnf"); + cnf = malloc(sizeof(char) * (strlen(TEST_DIR) + strlen("/maxscale.cnf") + 1)); + sprintf(cnf, "%s/maxscale.cnf", TEST_DIR); printf("Config: %s\n",cnf); @@ -89,7 +92,7 @@ int main(int argc, char** argv) } config_load(cnf); - + config_enable_feedback_task(); if ((fc = config_get_feedback_data()) == NULL) { FAILTEST("Configuration for Feedback was NULL."); diff --git a/server/core/test/testmodutil.c b/server/core/test/testmodutil.c index f8fe7ef24..39632ab6d 100644 --- a/server/core/test/testmodutil.c +++ b/server/core/test/testmodutil.c @@ -70,10 +70,11 @@ int test2() { GWBUF *buffer; -char len = 128; +unsigned int len = 128; char query[129]; - buffer = gwbuf_alloc(132); + /** Allocate space for the COM_QUERY header and payload */ + buffer = gwbuf_alloc(5 + 128); ss_info_dassert((buffer != NULL),"Buffer should not be null"); memset(query,';',128); diff --git a/server/core/test/testpoll.c b/server/core/test/testpoll.c index b67ad208f..61978b4f4 100644 --- a/server/core/test/testpoll.c +++ b/server/core/test/testpoll.c @@ -86,7 +86,7 @@ int result; sleep(10); poll_shutdown(); ss_dfprintf(stderr, "\t..done\nTidy up."); - dcb_free(dcb); + dcb_close(dcb); ss_dfprintf(stderr, "\t..done\n"); return 0; diff --git a/server/core/test/testservice.c b/server/core/test/testservice.c index 57542a64f..5fa7e0436 100644 --- a/server/core/test/testservice.c +++ b/server/core/test/testservice.c @@ -41,15 +41,6 @@ #include #include - -static bool success = false; - -int hup(DCB* dcb) -{ - success = true; - return 1; -} - /** * test1 Allocate a service and do lots of other things * @@ -64,17 +55,6 @@ int result; int argc = 3; init_test_env(NULL); -/* char* argv[] = */ -/* { */ -/* "log_manager", */ -/* "-j", */ -/* TEST_LOG_DIR, */ -/* NULL */ -/* }; */ - -/* skygw_logmanager_init(argc,argv); */ -/* poll_init(); */ -/* hkinit(); */ /* Service tests */ ss_dfprintf(stderr, @@ -98,9 +78,9 @@ init_test_env(NULL); result = serviceStart(service); skygw_log_sync_all(); ss_info_dassert(0 != result, "Start should succeed"); - result = serviceStop(service); + serviceStop(service); skygw_log_sync_all(); - ss_info_dassert(0 != result, "Stop should succeed"); + ss_info_dassert(service->state == SERVICE_STATE_STOPPED, "Stop should succeed"); result = serviceStartAll(); skygw_log_sync_all(); ss_info_dassert(0 != result, "Start all should succeed"); @@ -111,35 +91,26 @@ init_test_env(NULL); result = serviceStart(service); skygw_log_sync_all(); ss_info_dassert(0 != result, "Start should succeed"); - result = serviceStop(service); + serviceStop(service); skygw_log_sync_all(); - ss_info_dassert(0 != result, "Stop should succeed"); + ss_info_dassert(service->state == SERVICE_STATE_STOPPED, "Stop should succeed"); - if((dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER)) == NULL) - return 1; + if((dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER)) == NULL) + return 1; ss_info_dassert(dcb != NULL, "DCB allocation failed"); session = session_alloc(service,dcb); ss_info_dassert(session != NULL, "Session allocation failed"); - session->client->state = DCB_STATE_POLLING; - session->client->func.hangup = hup; + dcb->state = DCB_STATE_POLLING; sleep(15); - ss_info_dassert(success, "Session timeout failed"); + ss_info_dassert(dcb->state != DCB_STATE_POLLING, "Session timeout failed"); ss_dfprintf(stderr, "\t..done\nStopping Service."); - ss_info_dassert(0 != serviceStop(service), "Stop should succeed"); + serviceStop(service); + ss_info_dassert(service->state == SERVICE_STATE_STOPPED, "Stop should succeed"); ss_dfprintf(stderr, "\t..done\n"); - /** This is never used in MaxScale and will always fail due to service's - * stats.n_current value never being decremented */ -/* - - ss_dfprintf(stderr, "\t..done\nFreeing Service."); - ss_info_dassert(0 != service_free(service), "Free should succeed"); - ss_dfprintf(stderr, "\t..done\n"); - -*/ return 0; } diff --git a/server/modules/routing/readwritesplit/test/test_hints/rwsplit_hints.sh b/server/modules/routing/readwritesplit/test/test_hints/rwsplit_hints.sh index 67de5d4f6..46241d4c3 100755 --- a/server/modules/routing/readwritesplit/test/test_hints/rwsplit_hints.sh +++ b/server/modules/routing/readwritesplit/test/test_hints/rwsplit_hints.sh @@ -26,7 +26,7 @@ else TDIR=. fi -RUNCMD=mysql\ --host=$THOST\ -P$TPORT\ -u$TUSER\ -p$TPWD\ --unbuffered=true\ --disable-reconnect\ --silent\ --comment +RUNCMD="mysql --host=$THOST -P$TPORT -u$TUSER -p$TPWD --unbuffered=true --disable-reconnect --silent -c" i=0 while read -r LINE