From beacd524da5c35651dcb291de9164267fe09f8d9 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 29 Dec 2014 13:45:24 +0200 Subject: [PATCH] Fix to bug #662, http://bugs.mariadb.com/show_bug.cgi?id=662 dbusers.c: Added function for setting read, write and connection timeout values. Set default timeouts for getUsers. Defaults are listed in service.c gateway.c:shutdown_server is called whenever MaxScale is to be shut down. Added call for service_shutdown to shutdown_server. service.c:service_alloc: replaced malloc with calloc and removed unnecessary zero/NULL initialization statements as a consequence. serviceStart: Exit serviceStartPort loop if shutdown flag is set for the service. serviceStartAll: Exit serviceStart loop if shutdown flag is set for the service. service.c: Added service_shutdown which sets shutdown flag for each service found in allServices list. service.h: Added prototype for service_shutdown --- server/core/dbusers.c | 155 ++++++++++++++++++++++++++++++++------- server/core/gateway.c | 4 +- server/core/service.c | 43 +++++------ server/include/service.h | 2 + 4 files changed, 153 insertions(+), 51 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 35566de11..4d372079d 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -52,6 +52,10 @@ #include +#define DEFAULT_CONNECT_TIMEOUT 3 +#define DEFAULT_READ_TIMEOUT 1 +#define DEFAULT_WRITE_TIMEOUT 2 + #define USERS_QUERY_NO_ROOT " AND user NOT IN ('root')" #if 0 @@ -105,6 +109,11 @@ void *resource_fetch(HASHTABLE *, char *); int resource_add(HASHTABLE *, char *, char *); int resource_hash(char *); static int normalize_hostname(char *input_host, char *output_host); +static int gw_mysql_set_timeouts( + MYSQL* handle, + int read_timeout, + int write_timeout, + int connect_timeout); /** * Load the user/passwd form mysql.user table into the service users' hashtable @@ -429,7 +438,8 @@ getDatabases(SERVICE *service, MYSQL *con) * * @param service The current service * @param users The users table into which to load the users - * @return -1 on any error or the number of users inserted (0 means no users at all) + * @return -1 on any error or the number of users inserted + * (0 means no users at all) */ static int getUsers(SERVICE *service, USERS *users) @@ -446,15 +456,19 @@ getUsers(SERVICE *service, USERS *users) unsigned char hash[SHA_DIGEST_LENGTH]=""; char *users_data = NULL; int nusers = 0; - int users_data_row_len = MYSQL_USER_MAXLEN + MYSQL_HOST_MAXLEN + MYSQL_PASSWORD_LEN + sizeof(char) + MYSQL_DATABASE_MAXLEN; + int users_data_row_len = MYSQL_USER_MAXLEN + + MYSQL_HOST_MAXLEN + + MYSQL_PASSWORD_LEN + + sizeof(char) + + MYSQL_DATABASE_MAXLEN; int dbnames = 0; int db_grants = 0; - serviceGetUser(service, &service_user, &service_passwd); - - if (service_user == NULL || service_passwd == NULL) - return -1; - + if (serviceGetUser(service, &service_user, &service_passwd) == 0) + { + ss_dassert(service_passwd == NULL || service_user == NULL); + return -1; + } con = mysql_init(NULL); if (con == NULL) { @@ -464,13 +478,26 @@ getUsers(SERVICE *service, USERS *users) mysql_error(con)))); return -1; } + /** Set read, write and connect timeout values */ + if (gw_mysql_set_timeouts(con, + DEFAULT_READ_TIMEOUT, + DEFAULT_WRITE_TIMEOUT, + DEFAULT_CONNECT_TIMEOUT)) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : failed to set timeout values for backend " + "connection."))); + mysql_close(con); + return -1; + } if (mysql_options(con, MYSQL_OPT_USE_REMOTE_CONNECTION, NULL)) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : failed to set external connection. " - "It is needed for backend server connections. " - "Exiting."))); + "It is needed for backend server connections."))); + mysql_close(con); return -1; } /** @@ -486,12 +513,26 @@ getUsers(SERVICE *service, USERS *users) server = server->nextdb; } + if (service->svc_do_shutdown) + { + free(dpwd); + mysql_close(con); + return -1; + } + /* Try loading data from master server */ - if (server != NULL && (mysql_real_connect(con, server->name, service_user, dpwd, NULL, server->port, NULL, 0) != NULL)) { + if (server != NULL && + (mysql_real_connect(con, + server->name, service_user, + dpwd, + NULL, + server->port, + NULL, 0) != NULL)) + { LOGIF(LD, (skygw_log_write_flush( LOGFILE_DEBUG, - "Dbusers : Loading data from backend database with Master role [%s:%i] " - "for service [%s]", + "Dbusers : Loading data from backend database with " + "Master role [%s:%i] for service [%s]", server->name, server->port, service->name))); @@ -499,23 +540,32 @@ getUsers(SERVICE *service, USERS *users) /* load data from other servers via loop */ server = service->databases; - while (server != NULL && (mysql_real_connect(con, - server->name, - service_user, - dpwd, - NULL, - server->port, - NULL, - 0) == NULL)) + while (!service->svc_do_shutdown && + server != NULL && + (mysql_real_connect(con, + server->name, + service_user, + dpwd, + NULL, + server->port, + NULL, + 0) == NULL)) { server = server->nextdb; } - + + if (service->svc_do_shutdown) + { + free(dpwd); + mysql_close(con); + return -1; + } + if (server != NULL) { LOGIF(LD, (skygw_log_write_flush( LOGFILE_DEBUG, - "Dbusers : Loading data from backend database [%s:%i] " - "for service [%s]", + "Dbusers : Loading data from backend database " + "[%s:%i] for service [%s]", server->name, server->port, service->name))); @@ -535,9 +585,7 @@ getUsers(SERVICE *service, USERS *users) return -1; } - /* count users */ - - /* start with users and db grants for users */ + /** Count users. Start with users and db grants for users */ if (mysql_query(con, MYSQL_USERS_WITH_DB_COUNT)) { if (mysql_errno(con) != ER_TABLEACCESS_DENIED_ERROR) { /* This is an error we cannot handle, return */ @@ -1213,3 +1261,58 @@ int useorig = 0; return netmask; } + +/** + * Set read, write and connect timeout values for MySQL database connection. + * + * @param handle MySQL handle + * @param read_timeout Read timeout value in seconds + * @param write_timeout Write timeout value in seconds + * @param connect_timeout Connect timeout value in seconds + * + * @return 0 if succeed, 1 if failed + */ +static int gw_mysql_set_timeouts( + MYSQL* handle, + int read_timeout, + int write_timeout, + int connect_timeout) +{ + int rc; + + if ((rc = mysql_options(handle, + MYSQL_OPT_READ_TIMEOUT, + (void *)&read_timeout))) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : failed to set read timeout for backend " + "connection."))); + goto retblock; + } + + if ((rc = mysql_options(handle, + MYSQL_OPT_CONNECT_TIMEOUT, + (void *)&connect_timeout))) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : failed to set connect timeout for backend " + "connection."))); + goto retblock; + } + + if ((rc = mysql_options(handle, + MYSQL_OPT_WRITE_TIMEOUT, + (void *)&write_timeout))) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : failed to set write timeout for backend " + "connection."))); + goto retblock; + } + + retblock: + return rc; +} \ No newline at end of file diff --git a/server/core/gateway.c b/server/core/gateway.c index ee2290e28..a49f18e86 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -56,6 +56,7 @@ #include #include #include +#include #include #include @@ -1837,7 +1838,8 @@ return_main: void shutdown_server() { - poll_shutdown(); + service_shutdown(); + poll_shutdown(); hkshutdown(); memlog_flush_all(); log_flush_shutdown(); diff --git a/server/core/service.c b/server/core/service.c index 16f7b29ca..4ca702467 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -87,7 +87,6 @@ static void service_add_qualified_param( SERVICE* svc, CONFIG_PARAMETER* param); - /** * Allocate a new service for the gateway to support * @@ -102,7 +101,7 @@ service_alloc(const char *servname, const char *router) { SERVICE *service; - if ((service = (SERVICE *)malloc(sizeof(SERVICE))) == NULL) + if ((service = (SERVICE *)calloc(1, sizeof(SERVICE))) == NULL) return NULL; if ((service->router = load_module(router, MODULE_ROUTER)) == NULL) { @@ -132,27 +131,10 @@ SERVICE *service; free(service); return NULL; } - service->version_string = NULL; - memset(&service->stats, 0, sizeof(SERVICE_STATS)); - service->ports = NULL; service->stats.started = time(0); service->state = SERVICE_STATE_ALLOC; - service->credentials.name = NULL; - service->credentials.authdata = NULL; - service->enable_root = 0; - service->localhost_match_wildcard_host = 0; - service->routerOptions = NULL; - service->databases = NULL; - service->svc_config_param = NULL; - service->svc_config_version = 0; - service->filters = NULL; - service->n_filters = 0; - service->weightby = 0; - service->users = NULL; - service->resources = NULL; spinlock_init(&service->spin); spinlock_init(&service->users_table_spin); - memset(&service->rate_limit, 0, sizeof(SERVICE_REFRESH_RATE)); spinlock_acquire(&service_spin); service->next = allServices; @@ -360,7 +342,7 @@ int listeners = 0; } port = service->ports; - while (port) + while (!service->svc_do_shutdown && port) { listeners += serviceStartPort(service, port); port = port->next; @@ -408,16 +390,16 @@ SERVICE *ptr; int n = 0,i; ptr = allServices; - while (ptr) + while (ptr && !ptr->svc_do_shutdown) { n += (i = serviceStart(ptr)); if(i == 0) { LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "Error : Failed to start service '%s'.", - ptr->name))); + LOGFILE_ERROR, + "Error : Failed to start service '%s'.", + ptr->name))); } ptr = ptr->next; @@ -1422,3 +1404,16 @@ serviceEnableLocalhostMatchWildcardHost(SERVICE *service, int action) return 1; } + +void service_shutdown() +{ + SERVICE* svc; + spinlock_acquire(&service_spin); + svc = allServices; + while (svc != NULL) + { + svc->svc_do_shutdown = true; + svc = svc->next; + } + spinlock_release(&service_spin); +} \ No newline at end of file diff --git a/server/include/service.h b/server/include/service.h index 33109e742..3efd426ac 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -130,6 +130,7 @@ typedef struct service { CONFIG_PARAMETER* svc_config_param; /*< list of config params and values */ int svc_config_version; /*< Version number of configuration */ + bool svc_do_shutdown; /*< tells the service to exit loops etc. */ SPINLOCK users_table_spin; /**< The spinlock for users data refresh */ SERVICE_REFRESH_RATE @@ -186,4 +187,5 @@ extern void dprintService(DCB *, SERVICE *); extern void dListServices(DCB *); extern void dListListeners(DCB *); char* service_get_name(SERVICE* svc); +void service_shutdown(); #endif