From f74493d922adab277cad180a3f9591ef3a1d3c33 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 8 Sep 2014 21:44:23 +0300 Subject: [PATCH] server/modules/filter/Makefile: Fixed problem which prevented cleaning and compiling hintfilter library. server/core/config.c: Removed unused if..else block from config_get_valint. Changed it also to return value which indicates whether the operation succeed. Added config_get_valbool similar to config_get_valint. service.c:Added typelib-like struct and array of valid boolean values. Fixed parameter type test in service_set_param_value. Completed boolean type parameter handling. hintparser.c:Fixed error message for non-maxscale hints. readwritesplit.c:Added loading of configuration parameters from service to instance and from instance to each new session. Fixed routing condition in get_route_target. Modified get_route_target so that it takes also rw_read_sesvars_from_slaves and rw_write_sesvars_to_all as parameters. skygw_types.h: added array size counting macro. --- query_classifier/query_classifier.h | 3 - server/core/config.c | 72 ++++++++--- server/core/service.c | 16 +-- server/include/config.h | 9 +- server/modules/filter/Makefile | 8 +- server/modules/filter/hint/hintparser.c | 11 +- .../routing/readwritesplit/readwritesplit.c | 114 +++++++++++++----- utils/skygw_types.h | 1 + utils/skygw_utils.h | 1 + 9 files changed, 161 insertions(+), 74 deletions(-) diff --git a/query_classifier/query_classifier.h b/query_classifier/query_classifier.h index c92ac6286..4ad960524 100644 --- a/query_classifier/query_classifier.h +++ b/query_classifier/query_classifier.h @@ -25,9 +25,6 @@ Copyright SkySQL Ab EXTERN_C_BLOCK_BEGIN -bool allow_var_writes_to_slaves = false; -bool allow_var_reads_from_slaves = false; - /** * Query type for skygateway. * The meaninful difference is where operation is done and whether master data diff --git a/server/core/config.c b/server/core/config.c index f151cab97..47426eaf0 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -887,12 +888,15 @@ config_param_type_t config_get_paramtype( return param->qfd_param_type; } -int config_get_valint( +bool config_get_valint( + int* val, CONFIG_PARAMETER* param, const char* name, /*< if NULL examine current param only */ config_param_type_t ptype) -{ - int val = -1; /*< -1 indicates failure */ +{ + bool succp = false;; + + ss_dassert(ptype == COUNT_TYPE || ptype == PERCENT_TYPE); while (param) { @@ -900,29 +904,57 @@ int config_get_valint( { switch (ptype) { case COUNT_TYPE: - val = param->qfd.valcount; - goto return_val; + *val = param->qfd.valcount; + succp = true; + goto return_succp; case PERCENT_TYPE: - val = param->qfd.valpercent; - goto return_val; - - case BOOL_TYPE: - val = param->qfd.valbool; - goto return_val; - - default: - goto return_val; + *val = param->qfd.valpercent; + succp =true; + goto return_succp; + + default: + goto return_succp; } } - else if (name == NULL) - { - goto return_val; - } param = param->next; } -return_val: - return val; +return_succp: + return succp; +} + + +bool config_get_valbool( + bool* val, + CONFIG_PARAMETER* param, + const char* name, + config_param_type_t ptype) +{ + bool succp; + + ss_dassert(ptype == BOOL_TYPE); + + if (ptype != BOOL_TYPE) + { + succp = false; + goto return_succp; + } + + while (param) + { + if (name == NULL || !strncmp(param->name, name, MAX_PARAM_LEN)) + { + *val = param->qfd.valbool; + succp = true; + goto return_succp; + } + param = param->next; + } + succp = false; + +return_succp: + return succp; + } diff --git a/server/core/service.c b/server/core/service.c index bd2dee7c6..d84e2464e 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -64,8 +64,8 @@ typedef struct typelib_st { const char** tl_p_elems; } typelib_t; -static const char* bool_strings[9]= {"FALSE", "TRUE", "OFF", "ON", "N", "Y", "0", "1", "NO", "YES", 0}; -typelib_t bool_typelib = {array_nelems(bool_strings)-1, "bool_typelib", bool_strings}; +static const char* bool_strings[11]= {"FALSE", "TRUE", "OFF", "ON", "N", "Y", "0", "1", "NO", "YES", 0}; +typelib_t bool_type = {array_nelems(bool_strings)-1, "bool_type", bool_strings}; static SPINLOCK service_spin = SPINLOCK_INIT; static SERVICE *allServices = NULL; @@ -1026,7 +1026,7 @@ bool service_set_param_value ( bool valbool; bool succp = true; - if (type == PERCENT_TYPE || type == COUNT_TYPE) + if (PARAM_IS_TYPE(type,PERCENT_TYPE) ||PARAM_IS_TYPE(type,COUNT_TYPE)) { /** * Find out whether the value is numeric and ends with '%' or '\0' @@ -1090,7 +1090,7 @@ bool service_set_param_value ( { unsigned int rc; - rc = find_type(&bool_typelib, valstr, strlen(valstr)+1); + rc = find_type(&bool_type, valstr, strlen(valstr)+1); if (rc > 0) { @@ -1103,6 +1103,10 @@ bool service_set_param_value ( { valbool = true; } + /** add param to config */ + config_set_qualified_param(param, + (void *)&valbool, + BOOL_TYPE); } else { @@ -1111,7 +1115,7 @@ bool service_set_param_value ( } if (succp) { - config_set_qualified_param(param, (void *)&valbool, BOOL_TYPE); /*< add param to config */ + service_add_qualified_param(service, param); /*< add param to svc */ } return succp; } @@ -1152,8 +1156,6 @@ static int find_type( } return 0; } - - /** * Add qualified config parameter to SERVICE struct. diff --git a/server/include/config.h b/server/include/config.h index ca7d1ac5e..8dcb77c85 100644 --- a/server/include/config.h +++ b/server/include/config.h @@ -99,9 +99,16 @@ bool config_set_qualified_param( config_param_type_t type); -int config_get_valint( +bool config_get_valint( + int* val, CONFIG_PARAMETER* param, const char* name, /*< if NULL examine current param only */ config_param_type_t ptype); +bool config_get_valbool( + bool* val, + CONFIG_PARAMETER* param, + const char* name, /*< if NULL examine current param only */ + config_param_type_t ptype); + #endif diff --git a/server/modules/filter/Makefile b/server/modules/filter/Makefile index f4ad4cac3..49a5e9f23 100644 --- a/server/modules/filter/Makefile +++ b/server/modules/filter/Makefile @@ -48,7 +48,7 @@ MQOBJ=$(MQSRCS:.c=.o) SRCS=$(TESTSRCS) $(QLASRCS) $(REGEXSRCS) $(TOPNSRCS) $(TEESRCS) OBJ=$(SRCS:.c=.o) LIBS=$(UTILSPATH)/skygw_utils.o -lssl -llog_manager -MODULES= libtestfilter.so libqlafilter.so libregexfilter.so libtopfilter.so libtee.so +MODULES= libtestfilter.so libqlafilter.so libregexfilter.so libtopfilter.so libhintfilter.so libtee.so ifndef BUILD_RABBITMQ BUILD_RABBITMQ=Y @@ -81,7 +81,7 @@ libtee.so: $(TEEOBJ) $(CC) $(LDFLAGS) $(TEEOBJ) $(LIBS) -o $@ libhintfilter.so: -# (cd hint; touch depend.mk ; make; cp $@ ..) + (cd hint; touch depend.mk ; make; cp $@ ..) .c.o: $(CC) $(CFLAGS) $< -o $@ @@ -92,12 +92,12 @@ clean: tags: ctags $(SRCS) $(HDRS) -# (cd hint; touch depend.mk; make tags) + (cd hint; touch depend.mk; make tags) depend: @rm -f depend.mk cc -M $(CFLAGS) $(SRCS) > depend.mk -# (cd hint; touch depend.mk; make depend) + (cd hint; touch depend.mk; make depend) install: $(MODULES) install -D $(MODULES) $(DEST)/modules diff --git a/server/modules/filter/hint/hintparser.c b/server/modules/filter/hint/hintparser.c index 2c6624328..205580c5a 100644 --- a/server/modules/filter/hint/hintparser.c +++ b/server/modules/filter/hint/hintparser.c @@ -231,18 +231,9 @@ HINT_MODE mode = HM_EXECUTE; goto retblock; } + /** This is not MaxScale hint because it doesn't start with 'maxscale' */ if (tok->token != TOK_MAXSCALE) { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Error : Invalid hint string '%s'. Hint should start " - "with keyword 'maxscale'. Hint ignored.", - token_get_keyword(tok)))); - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Invalid hint string '%s'. Hint should start " - "with keyword 'maxscale'. Hint ignored.", - token_get_keyword(tok)))); token_free(tok); goto retblock; } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index c043f4711..d9616872d 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -100,10 +100,11 @@ static backend_ref_t* get_bref_from_dcb(ROUTER_CLIENT_SES* rses, DCB* dcb); static route_target_t get_route_target ( skygw_query_type_t qtype, + bool read_sesvars_from_slaves, + bool write_sesvars_to_all, bool trx_active, HINT* hint); - static uint8_t getCapabilities (ROUTER* inst, void* router_session); #if defined(NOT_USED) @@ -366,7 +367,8 @@ static void refreshInstance( { CONFIG_PARAMETER* param; bool refresh_single; - + config_param_type_t paramtype; + if (singleparam != NULL) { param = singleparam; @@ -377,39 +379,91 @@ static void refreshInstance( param = router->service->svc_config_param; refresh_single = false; } - + paramtype = config_get_paramtype(param); + while (param != NULL) { - config_param_type_t paramtype; - - paramtype = config_get_paramtype(param); - if (paramtype == COUNT_TYPE) { if (strncmp(param->name, "max_slave_connections", MAX_PARAM_LEN) == 0) { + int val; + bool succp; + router->rwsplit_config.rw_max_slave_conn_percent = 0; - router->rwsplit_config.rw_max_slave_conn_count = - config_get_valint(param, NULL, paramtype); + + succp = config_get_valint(&val, param, NULL, paramtype); + + if (succp) + { + router->rwsplit_config.rw_max_slave_conn_count = val; + } } else if (strncmp(param->name, "max_slave_replication_lag", MAX_PARAM_LEN) == 0) { - router->rwsplit_config.rw_max_slave_replication_lag = - config_get_valint(param, NULL, paramtype); - } + int val; + bool succp; + + succp = config_get_valint(&val, param, NULL, paramtype); + + if (succp) + { + router->rwsplit_config.rw_max_slave_replication_lag = val; + } + } } else if (paramtype == PERCENT_TYPE) { if (strncmp(param->name, "max_slave_connections", MAX_PARAM_LEN) == 0) { + int val; + bool succp; + router->rwsplit_config.rw_max_slave_conn_count = 0; - router->rwsplit_config.rw_max_slave_conn_percent = - config_get_valint(param, NULL, paramtype); + + succp = config_get_valint(&val, param, NULL, paramtype); + + if (succp) + { + router->rwsplit_config.rw_max_slave_conn_percent = val; + } } } + if (paramtype == BOOL_TYPE) + { + if (strncmp(param->name, + "read_ses_variables_from_slaves", + MAX_PARAM_LEN) == 0) + { + bool val; + bool succp; + + succp = config_get_valbool(&val, param, NULL, paramtype); + + if (succp) + { + router->rwsplit_config.rw_read_sesvars_from_slaves = val; + } + } + else if (strncmp(param->name, + "write_ses_variables_to_all", + MAX_PARAM_LEN) == 0) + { + bool val; + bool succp; + + succp = config_get_valbool(&val, param, NULL, paramtype); + + if (succp) + { + router->rwsplit_config.rw_write_sesvars_to_all = val; + } + } + } + if (refresh_single) { break; @@ -417,7 +471,7 @@ static void refreshInstance( param = param->next; } -#if defined(NOT_USED) /*< can't read monitor config parameters */ +#if (NOT_USED) /*< can't read monitor config parameters */ if ((*router->servers)->backend_server->rlag == -2) { rlag_enabled = false; @@ -459,6 +513,7 @@ static void refreshInstance( } } #endif /*< NOT_USED */ + } /** @@ -1130,6 +1185,8 @@ return_succp: static route_target_t get_route_target ( skygw_query_type_t qtype, bool trx_active, + bool read_ses_variables_from_slaves, + bool write_ses_variables_to_all, HINT* hint) { route_target_t target; @@ -1137,11 +1194,10 @@ static route_target_t get_route_target ( * These queries are not affected by hints */ if (!trx_active && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || + (QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || /** Configured to allow writing variables to all nodes */ - (allow_var_writes_to_slaves && + (write_ses_variables_to_all && (QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE))))) { @@ -1161,7 +1217,7 @@ static route_target_t get_route_target ( /** First set expected targets before evaluating hints */ if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || /** Configured to allow reading variables from slaves */ - (allow_var_reads_from_slaves && + (read_ses_variables_from_slaves && (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ)))) @@ -1170,7 +1226,7 @@ static route_target_t get_route_target ( } else if (QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || /** Configured not to allow reading variables from slaves */ - (!allow_var_reads_from_slaves && + (!read_ses_variables_from_slaves && (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ)))) { @@ -1250,13 +1306,13 @@ static route_target_t get_route_target ( QUERY_IS_TYPE(qtype, QUERY_TYPE_MASTER_READ) || QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) && - !allow_var_writes_to_slaves) || + !write_ses_variables_to_all) || (QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) && - !allow_var_writes_to_slaves) || + !write_ses_variables_to_all) || (QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ) && - !allow_var_writes_to_slaves) || + !write_ses_variables_to_all) || (QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE) && - !allow_var_writes_to_slaves) || + !write_ses_variables_to_all) || QUERY_IS_TYPE(qtype, QUERY_TYPE_BEGIN_TRX) || QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT) || @@ -1606,8 +1662,7 @@ static int routeQuery( inst->stats.n_queries++; master_dcb = router_cli_ses->rses_master_ref->bref_dcb; - CHK_DCB(master_dcb); - + CHK_DCB(master_dcb); switch(packet_type) { case MYSQL_COM_QUIT: /*< 1 QUIT will close all sessions */ @@ -1695,8 +1750,7 @@ static int routeQuery( { router_cli_ses->rses_autocommit_enabled = true; router_cli_ses->rses_transaction_active = false; - } - + } /** * Find out where to route the query. Result may not be clear; it is @@ -1716,7 +1770,9 @@ static int routeQuery( * eventually to master */ route_target = get_route_target(qtype, - router_cli_ses->rses_transaction_active, + router_cli_ses->rses_transaction_active, + router_cli_ses->rses_config.rw_read_sesvars_from_slaves, + router_cli_ses->rses_config.rw_write_sesvars_to_all, querybuf->hint); if (TARGET_IS_ALL(route_target)) diff --git a/utils/skygw_types.h b/utils/skygw_types.h index cccf55f2b..a82db80ab 100644 --- a/utils/skygw_types.h +++ b/utils/skygw_types.h @@ -45,5 +45,6 @@ #endif #define MAX_ERROR_MSG PATH_MAX +#define array_nelems(a) ((uint)(sizeof(a)/sizeof(a[0]))) #endif /* SKYGW_TYPES_H */ diff --git a/utils/skygw_utils.h b/utils/skygw_utils.h index a50dd08b6..a54859392 100644 --- a/utils/skygw_utils.h +++ b/utils/skygw_utils.h @@ -83,6 +83,7 @@ typedef enum { THR_INIT, THR_RUNNING, THR_STOPPED, THR_DONE } skygw_thr_state_t; typedef enum { MES_RC_FAIL, MES_RC_SUCCESS, MES_RC_TIMEOUT } skygw_mes_rc_t; EXTERN_C_BLOCK_BEGIN + slist_cursor_t* slist_init(void); void slist_done(slist_cursor_t* c);