From d5c38b93f6a57bc58bf038dcc744bd4cd8511b76 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 29 Oct 2015 20:24:26 +0200 Subject: [PATCH] Fix to MXS-431: https://mariadb.atlassian.net/browse/MXS-431 Replaced the use of the shared MySQLSession structure with an internal variable that tracks the currently active database. --- server/modules/include/schemarouter.h | 1 + server/modules/include/sharding_common.h | 4 +- server/modules/include/shardrouter.h | 1 + .../routing/schemarouter/schemarouter.c | 48 +++++++------------ .../routing/schemarouter/sharding_common.c | 10 ++-- .../routing/schemarouter/shardrouter.c | 19 +------- 6 files changed, 27 insertions(+), 56 deletions(-) diff --git a/server/modules/include/schemarouter.h b/server/modules/include/schemarouter.h index b988df8bd..d583dd5c3 100644 --- a/server/modules/include/schemarouter.h +++ b/server/modules/include/schemarouter.h @@ -323,6 +323,7 @@ struct router_client_session { struct router_client_session* next; /*< List of router sessions */ shard_map_t* shardmap; /*< Database hash containing names of the databases mapped to the servers that contain them */ char connect_db[MYSQL_DATABASE_MAXLEN+1]; /*< Database the user was trying to connect to */ + char current_db[MYSQL_DATABASE_MAXLEN + 1]; /*< Current active database */ init_mask_t init; /*< Initialization state bitmask */ GWBUF* queue; /*< Query that was received before the session was ready */ DCB* dcb_route; /*< Internal DCB used to trigger re-routing of buffers */ diff --git a/server/modules/include/sharding_common.h b/server/modules/include/sharding_common.h index dd30a74ea..b0d917f0a 100644 --- a/server/modules/include/sharding_common.h +++ b/server/modules/include/sharding_common.h @@ -12,8 +12,6 @@ bool extract_database(GWBUF* buf, char* str); void create_error_reply(char* fail_str,DCB* dcb); -bool change_current_db(MYSQL_session* mysql_session, - HASHTABLE* dbhash, - GWBUF* buf); +bool change_current_db(char* dest, HASHTABLE* dbhash, GWBUF* buf); #endif diff --git a/server/modules/include/shardrouter.h b/server/modules/include/shardrouter.h index 63572671e..3fc70fcc1 100644 --- a/server/modules/include/shardrouter.h +++ b/server/modules/include/shardrouter.h @@ -204,6 +204,7 @@ struct router_client_session { SESSION* session; GWBUF* queue; char connect_db[MYSQL_DATABASE_MAXLEN+1]; /*< Database the user was trying to connect to */ + char current_db[MYSQL_DATABASE_MAXLEN + 1]; /*< Current active database */ shard_init_mask_t init; /*< Initialization state bitmask */ #if defined(SS_DEBUG) skygw_chk_t rses_chk_tail; diff --git a/server/modules/routing/schemarouter/schemarouter.c b/server/modules/routing/schemarouter/schemarouter.c index d2e076254..e7e378021 100644 --- a/server/modules/routing/schemarouter/schemarouter.c +++ b/server/modules/routing/schemarouter/schemarouter.c @@ -583,9 +583,9 @@ char* get_shard_target_name(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client, if(tmp == NULL) { - rval = (char*) hashtable_fetch(ht, client->rses_mysql_session->db); + rval = (char*) hashtable_fetch(ht, client->current_db); skygw_log_write(LOGFILE_TRACE,"schemarouter: SHOW TABLES query, current database '%s' on server '%s'", - client->rses_mysql_session->db,rval); + client->current_db,rval); } else { @@ -613,17 +613,17 @@ char* get_shard_target_name(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client, } } - if(rval == NULL && !has_dbs && client->rses_mysql_session->db[0] != '\0') + if(rval == NULL && !has_dbs && client->current_db[0] != '\0') { /** * If the target name has not been found and the session has an * active database, set is as the target */ - rval = (char*) hashtable_fetch(ht, client->rses_mysql_session->db); + rval = (char*) hashtable_fetch(ht, client->current_db); if(rval) { - skygw_log_write(LOGFILE_TRACE,"schemarouter: Using active database '%s'",client->rses_mysql_session->db); + skygw_log_write(LOGFILE_TRACE,"schemarouter: Using active database '%s'",client->current_db); } } @@ -1585,7 +1585,7 @@ ROUTER* instance, rses_property_t* rses_prop_tmp; rses_prop_tmp = router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES]; - dbname = router_cli_ses->rses_mysql_session->db; + dbname = router_cli_ses->current_db; if (is_drop_table_query(querybuf)) { @@ -1643,7 +1643,7 @@ ROUTER* instance, rses_property_t* rses_prop_tmp; rses_prop_tmp = router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES]; - dbname = router_cli_ses->rses_mysql_session->db; + dbname = router_cli_ses->current_db; if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || QUERY_IS_TYPE(qtype, QUERY_TYPE_LOCAL_READ) || @@ -1722,7 +1722,7 @@ void check_create_tmp_table( HASHTABLE* h; rses_prop_tmp = router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES]; - dbname = router_cli_ses->rses_mysql_session->db; + dbname = router_cli_ses->current_db; if (QUERY_IS_TYPE(type, QUERY_TYPE_CREATE_TMP_TABLE)) @@ -2248,7 +2248,7 @@ static int routeQuery( op == QUERY_OP_CHANGE_DB) { spinlock_acquire(&router_cli_ses->shardmap->lock); - change_successful = change_current_db(router_cli_ses->rses_mysql_session, + change_successful = change_current_db(router_cli_ses->current_db, router_cli_ses->shardmap->hash, querybuf); spinlock_release(&router_cli_ses->shardmap->lock); @@ -2259,6 +2259,10 @@ static int routeQuery( difftime(now,router_cli_ses->rses_config.last_refresh) > router_cli_ses->rses_config.refresh_min_interval) { + spinlock_acquire(&router_cli_ses->shardmap->lock); + router_cli_ses->shardmap->state = SHMAP_STALE; + spinlock_release(&router_cli_ses->shardmap->lock); + rses_begin_locked_router_action(router_cli_ses); router_cli_ses->rses_config.last_refresh = now; @@ -2319,13 +2323,13 @@ static int routeQuery( route_target = TARGET_UNDEFINED; spinlock_acquire(&router_cli_ses->shardmap->lock); - tname = hashtable_fetch(router_cli_ses->shardmap->hash,router_cli_ses->rses_mysql_session->db); + tname = hashtable_fetch(router_cli_ses->shardmap->hash,router_cli_ses->current_db); spinlock_release(&router_cli_ses->shardmap->lock); if(tname) { skygw_log_write(LOGFILE_TRACE,"schemarouter: INIT_DB for database '%s' on server '%s'", - router_cli_ses->rses_mysql_session->db,tname); + router_cli_ses->current_db,tname); route_target = TARGET_NAMED_SERVER; } else @@ -2361,9 +2365,9 @@ static int routeQuery( if( (tname == NULL && packet_type != MYSQL_COM_INIT_DB && - router_cli_ses->rses_mysql_session->db[0] == '\0') || + router_cli_ses->current_db[0] == '\0') || packet_type == MYSQL_COM_FIELD_LIST || - (router_cli_ses->rses_mysql_session->db[0] != '\0')) + (router_cli_ses->current_db[0] != '\0')) { /** * No current database and no databases in query or @@ -2806,7 +2810,7 @@ static void clientReply(ROUTER* instance, router_cli_ses->connect_db, router_cli_ses->rses_client_dcb->session); router_cli_ses->init &= ~INIT_USE_DB; - strcpy(router_cli_ses->rses_mysql_session->db, router_cli_ses->connect_db); + strcpy(router_cli_ses->current_db, router_cli_ses->connect_db); ss_dassert(router_cli_ses->init == INIT_READY); if (router_cli_ses->queue) @@ -3728,22 +3732,6 @@ static bool execute_sescmd_in_backend( sescmd_cursor_clone_querybuf(scur)); break; - case MYSQL_COM_INIT_DB: - { - /** - * Record database name and store to session. - */ - GWBUF* tmpbuf; - MYSQL_session* data; - unsigned int qlen; - - data = dcb->session->data; - tmpbuf = scur->scmd_cur_cmd->my_sescmd_buf; - qlen = MYSQL_GET_PACKET_LEN((unsigned char*)tmpbuf->start); - memset(data->db,0,MYSQL_DATABASE_MAXLEN+1); - strncpy(data->db,tmpbuf->start+5,qlen - 1); - } - /** Fallthrough */ case MYSQL_COM_QUERY: default: /** diff --git a/server/modules/routing/schemarouter/sharding_common.c b/server/modules/routing/schemarouter/sharding_common.c index cf6bf2c12..067edae3f 100644 --- a/server/modules/routing/schemarouter/sharding_common.c +++ b/server/modules/routing/schemarouter/sharding_common.c @@ -100,17 +100,17 @@ void create_error_reply(char* fail_str,DCB* dcb) } /** - * Read new database name from MYSQL_COM_INIT_DB packet or a literal USE ... COM_QUERY packet, check that it exists - * in the hashtable and copy its name to MYSQL_session. + * Read new database name from MYSQL_COM_INIT_DB packet or a literal USE ... COM_QUERY + * packet, check that it exists in the hashtable and copy its name to MYSQL_session. * - * @param mysql_session The MySQL session structure + * @param dest Destination where the database name will be written * @param dbhash Hashtable containing valid databases * @param buf Buffer containing the database change query * * @return true if new database is set, false if non-existent database was tried * to be set */ -bool change_current_db(MYSQL_session* mysql_session, +bool change_current_db(char* dest, HASHTABLE* dbhash, GWBUF* buf) { @@ -139,7 +139,7 @@ bool change_current_db(MYSQL_session* mysql_session, } else { - strncpy(mysql_session->db,db,MYSQL_DATABASE_MAXLEN); + strncpy(dest,db,MYSQL_DATABASE_MAXLEN); skygw_log_write(LOGFILE_TRACE,"change_current_db: database is on server: '%s'.",target); succp = true; goto retblock; diff --git a/server/modules/routing/schemarouter/shardrouter.c b/server/modules/routing/schemarouter/shardrouter.c index 9efcd20e9..a9b033962 100644 --- a/server/modules/routing/schemarouter/shardrouter.c +++ b/server/modules/routing/schemarouter/shardrouter.c @@ -1698,7 +1698,7 @@ routeQuery(ROUTER* instance, if(packet_type == MYSQL_COM_INIT_DB) { - if(!(change_successful = change_current_db(router_cli_ses->rses_mysql_session, + if(!(change_successful = change_current_db(router_cli_ses->current_db, router_cli_ses->dbhash, querybuf))) { @@ -2471,23 +2471,6 @@ execute_sescmd_in_backend(SUBSERVICE* subsvc) rc = SESSION_ROUTE_QUERY(subsvc->session,sescmd_cursor_clone_querybuf(scur)); break; - case MYSQL_COM_INIT_DB: - { - /** - * Record database name and store to session. - */ - GWBUF* tmpbuf; - MYSQL_session* data; - unsigned int qlen; - - data = subsvc->session->data; - tmpbuf = scur->scmd_cur_cmd->my_sescmd_buf; - qlen = MYSQL_GET_PACKET_LEN((unsigned char*) tmpbuf->start); - memset(data->db, 0, MYSQL_DATABASE_MAXLEN + 1); - strncpy(data->db, tmpbuf->start + 5, qlen - 1); - rc = SESSION_ROUTE_QUERY(subsvc->session,sescmd_cursor_clone_querybuf(scur)); - } - /** Fallthrough */ case MYSQL_COM_QUERY: default: /**