From a1361d9c9ec32bdedea65b2d2621b1c302be075a Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Fri, 2 May 2014 16:35:40 +0300 Subject: [PATCH] Related to MAX-95, added code which makes it possible to change max_slave_connections parameter value during run time of MaxScale. The change will be effective in rwsplit sessions created after the configuration modification and reload. Note that in order to config modification to be effective, the user must connect the debug interface of MaxScale (telnet) and execute 'reload config'. In practice, 'reload config' reads config file and updates config parameters of rwsplit router service. In addition to that there is a version number indicating what generation of configuration service holds. When router instance is created (when MaxScale is started) service's config version is copied to router intance. Whenever new client connection (rwsplit session) is started, router instance's config version is checked against that of service's. If versions differ, service's config data is copied to router instance. New session will be started with router instance's config values. --- server/core/config.c | 40 +++++- server/core/service.c | 19 ++- server/core/session.c | 4 +- server/include/service.h | 1 + server/modules/include/readwritesplit.h | 1 + .../routing/readwritesplit/readwritesplit.c | 136 ++++++++++++++---- 6 files changed, 172 insertions(+), 29 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index c35d75ddd..3ddfde9da 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -205,7 +205,7 @@ int error_count = 0; char *enable_root_user = config_get_value(obj->parameters, "enable_root_user"); - if (obj->element == NULL) + if (obj->element == NULL) /*< if module load failed */ { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -752,6 +752,8 @@ SERVER *server; char *user; char *auth; char *enable_root_user; + char* max_slave_conn_str; + enable_root_user = config_get_value(obj->parameters, "enable_root_user"); @@ -765,6 +767,42 @@ SERVER *server; auth); if (enable_root_user) serviceEnableRootUser(service, atoi(enable_root_user)); + max_slave_conn_str = + config_get_value( + obj->parameters, + "max_slave_connections"); + + if (max_slave_conn_str != NULL) + { + CONFIG_PARAMETER* param; + bool succp; + + param = config_get_param(obj->parameters, + "max_slave_connections"); + + succp = service_set_slave_conn_limit( + service, + param, + max_slave_conn_str, + COUNT_ATMOST); + + if (!succp) + { + LOGIF(LM, (skygw_log_write( + LOGFILE_MESSAGE, + "* Warning : invalid value type " + "for parameter \'%s.%s = %s\'\n\tExpected " + "type is either for slave connection " + "count or\n\t%% for specifying the " + "maximum percentage of available the " + "slaves that will be connected.", + ((SERVICE*)obj->element)->name, + param->name, + param->value))); + } + } + + } obj->element = service; diff --git a/server/core/service.c b/server/core/service.c index b951a4c35..cf2c91d2a 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -106,6 +106,7 @@ SERVICE *service; service->routerOptions = NULL; service->databases = NULL; service->svc_config_param = NULL; + service->svc_config_version = 0; spinlock_init(&service->spin); spinlock_init(&service->users_table_spin); memset(&service->rate_limit, 0, sizeof(SERVICE_REFRESH_RATE)); @@ -855,16 +856,30 @@ static void service_add_qualified_param( spinlock_acquire(&svc->spin); p = &svc->svc_config_param; - + if ((*p) != NULL) { - while ((*p)->next != NULL) *p = (*p)->next; + do + { + /** If duplicate is found, latter remains */ + if (strncasecmp(param->name, + (*p)->name, + strlen(param->name)) == 0) + { + *p = config_clone_param(param); + break; + } + } + while ((*p)->next != NULL); + (*p)->next = config_clone_param(param); } else { (*p) = config_clone_param(param); } + /** Increment service's configuration version */ + atomic_add(&svc->svc_config_version, 1); (*p)->next = NULL; spinlock_release(&svc->spin); } \ No newline at end of file diff --git a/server/core/session.c b/server/core/session.c index 062c63f27..fd880939e 100644 --- a/server/core/session.c +++ b/server/core/session.c @@ -138,8 +138,8 @@ session_alloc(SERVICE *service, DCB *client_dcb) session = NULL; LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : Failed to create router " - "client session. Freeing allocated resources."))); + "Error : Failed to create %s session.", + service->name))); goto return_session; } diff --git a/server/include/service.h b/server/include/service.h index ea44c7f37..97c29b0a6 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -116,6 +116,7 @@ typedef struct service { struct users *users; /**< The user data for this service */ int enable_root; /**< Allow root user access */ CONFIG_PARAMETER* svc_config_param; /*< list of config params and values */ + int svc_config_version; /*< Version number of configuration */ SPINLOCK users_table_spin; /**< The spinlock for users data refresh */ SERVICE_REFRESH_RATE diff --git a/server/modules/include/readwritesplit.h b/server/modules/include/readwritesplit.h index 408743c25..c8479cb11 100644 --- a/server/modules/include/readwritesplit.h +++ b/server/modules/include/readwritesplit.h @@ -191,6 +191,7 @@ typedef struct router_instance { BACKEND** servers; /*< Backend servers */ BACKEND* master; /*< NULL or pointer */ rwsplit_config_t rwsplit_config; /*< expanded config info from SERVICE */ + int rwsplit_version;/*< version number for router's config */ unsigned int bitmask; /*< Bitmask to apply to server->status */ unsigned int bitvalue; /*< Required value of server->status */ ROUTER_STATS stats; /*< Statistics for this router */ diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 7283f98a1..965f1af01 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -167,6 +167,10 @@ static bool route_session_write( unsigned char packet_type, skygw_query_type_t qtype); +static void refreshInstance( + ROUTER_INSTANCE* router, + CONFIG_PARAMETER* param); + static SPINLOCK instlock; static ROUTER_INSTANCE* instances; @@ -208,6 +212,35 @@ ROUTER_OBJECT* GetModuleObject() return &MyObject; } +static void refreshInstance( + ROUTER_INSTANCE* router, + CONFIG_PARAMETER* param) +{ + 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) + { + router->rwsplit_config.rw_max_slave_conn_percent = 0; + router->rwsplit_config.rw_max_slave_conn_count = + config_get_valint(param, NULL, paramtype); + } + } + else if (paramtype == PERCENT_TYPE) + { + if (strncmp(param->name, "max_slave_connections", MAX_PARAM_LEN) == 0) + { + router->rwsplit_config.rw_max_slave_conn_count = 0; + router->rwsplit_config.rw_max_slave_conn_percent = + config_get_valint(param, NULL, paramtype); + } + } +} + + /** * Create an instance of read/write statemtn router within the MaxScale. * @@ -226,7 +259,6 @@ static ROUTER* createInstance( int nservers; int i; CONFIG_PARAMETER* param; - config_param_type_t paramtype; if ((router = calloc(1, sizeof(ROUTER_INSTANCE))) == NULL) { return NULL; @@ -322,25 +354,15 @@ static ROUTER* createInstance( } } /** - * Copy config parameter value from service struct. This becomes the - * default value for every new rwsplit router session. + * Copy all config parameters from service to router instance. + * Finally, copy version number to indicate that configs match. */ param = config_get_param(service->svc_config_param, "max_slave_connections"); if (param != NULL) { - paramtype = config_get_paramtype(param); - - if (paramtype == COUNT_TYPE) - { - router->rwsplit_config.rw_max_slave_conn_count = - config_get_valint(param, NULL, paramtype); - } - else if (paramtype == PERCENT_TYPE) - { - router->rwsplit_config.rw_max_slave_conn_percent = - config_get_valint(param, NULL, paramtype); - } + refreshInstance(router, param); + router->rwsplit_version = service->svc_config_version; } /** * We have completed the creation of the router data, so now @@ -379,8 +401,8 @@ static void* newSession( int max_nslaves; /*< max # of slaves used in this session */ int conf_max_nslaves; /*< value from configuration file */ int i; - static int min_nservers = 1; /*< hard-coded for now */ - + const int min_nservers = 1; /*< hard-coded for now */ + client_rses = (ROUTER_CLIENT_SES *)calloc(1, sizeof(ROUTER_CLIENT_SES)); if (client_rses == NULL) @@ -392,9 +414,25 @@ static void* newSession( client_rses->rses_chk_top = CHK_NUM_ROUTER_SES; client_rses->rses_chk_tail = CHK_NUM_ROUTER_SES; #endif + /** + * If service config has been changed, reload config from service to + * router instance first. + */ + spinlock_acquire(&router->lock); + if (router->service->svc_config_version > router->rwsplit_version) + { + CONFIG_PARAMETER* param = router->service->svc_config_param; + + while (param != NULL) + { + refreshInstance(router, param); + param = param->next; + } + router->rwsplit_version = router->service->svc_config_version; + } /** Copy config struct from router instance */ client_rses->rses_config = router->rwsplit_config; - + spinlock_release(&router->lock); /** * Set defaults to session variables. */ @@ -406,9 +444,54 @@ static void* newSession( while (*(b++) != NULL) router_nservers++; /** With too few servers session is not created */ - if (router_nservers < min_nservers) + if (router_nservers < min_nservers || + MAX(client_rses->rses_config.rw_max_slave_conn_count, + (router_nservers*client_rses->rses_config.rw_max_slave_conn_percent)/100) + < min_nservers) { - /** log this */ + if (router_nservers < min_nservers) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Unable to start %s service. There are " + "too few backend servers available. Found %d " + "when %d is required.", + router->service->name, + router_nservers, + min_nservers))); + } + else + { + double pct = client_rses->rses_config.rw_max_slave_conn_percent/100; + double nservers = (double)router_nservers*pct; + + if (client_rses->rses_config.rw_max_slave_conn_count < + min_nservers) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Unable to start %s service. There are " + "too few backend servers configured in " + "MaxScale.cnf. Found %d when %d is required.", + router->service->name, + client_rses->rses_config.rw_max_slave_conn_count, + min_nservers))); + } + if (nservers < min_nservers) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Unable to start %s service. There are " + "too few backend servers configured in " + "MaxScale.cnf. Found %d%% when at least %.0f%% " + "would be required.", + router->service->name, + client_rses->rses_config.rw_max_slave_conn_percent, + min_nservers/(((double)router_nservers)/100)))); + } + } + free(client_rses); + client_rses = NULL; goto return_rses; } /** @@ -418,8 +501,9 @@ static void* newSession( if (backend_ref == NULL) { - /** log this */ + /** log this */ free(client_rses); + client_rses = NULL; goto return_rses; } /** @@ -498,9 +582,13 @@ static void* newSession( router->connections = client_rses; spinlock_release(&router->lock); - CHK_CLIENT_RSES(client_rses); - -return_rses: +return_rses: +#if defined(SS_DEBUG) + if (client_rses != NULL) + { + CHK_CLIENT_RSES(client_rses); + } +#endif return (void *)client_rses; }