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; }