From 33d8db02a6aef1cacb4a4c8b0e7e4a4f0bd0680f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Thu, 9 Jul 2020 23:45:03 +0200 Subject: [PATCH] Change the mapping logic for SESSION_TRACK_GTIDS Changing the mapping logic in SESSION_TRACK_GTIDS in frontend and backend connections. Up to now, frontend and backend `SESSION_TRACK_GTIDSs match. This is now changed: * backend connections are by default set to `mysql-default_session_track_gtids` * if `mysql-default_session_track_gtids=OFF` (the default) , `session_track_gtids` is not changed on backend * if the client asks for `session_track_gtids=OFF` , proxysql ignores it (it just acknowledge it) * if the client asks for `session_track_gtids=OWN_GTID` , proxysql will apply it * if the client asks for `session_track_gtids=ALL_GTIDS` , proxysql will switch to OWN_GTID and generate a warning * if the backend doesn't support `session_track_gtids` (for example in MySQL 5.5 and MySQL 5.6), proxysql won't apply it. It knows checking server capabilities This commit also deprecates function `MySQL_Session::handler_again___verify_backend__generic_variable()` ProxySQL is now also able to know when proxysql binlog reader is perhaps in use tracking all gtid_port in mysql_servers. This feature in future will be used to deprecate `mysql-default_session_track_gtids` --- include/MySQL_HostGroups_Manager.h | 7 ++ include/MySQL_Session.h | 1 - include/MySQL_Thread.h | 1 + lib/MySQL_HostGroups_Manager.cpp | 19 +++- lib/MySQL_Session.cpp | 146 +++++++++++++---------------- lib/MySQL_Thread.cpp | 3 + 6 files changed, 96 insertions(+), 81 deletions(-) diff --git a/include/MySQL_HostGroups_Manager.h b/include/MySQL_HostGroups_Manager.h index 857a538a5..7b4ca3672 100644 --- a/include/MySQL_HostGroups_Manager.h +++ b/include/MySQL_HostGroups_Manager.h @@ -4,6 +4,7 @@ #include "cpp.h" #include "proxysql_gtid.h" +#include #include #include #include @@ -502,6 +503,12 @@ class MySQL_HostGroups_Manager { void p_update_mysql_error_counter(p_mysql_error_type err_type, unsigned int hid, char* address, uint16_t port, unsigned int code); wqueue queue; + // has_gtid_port is set to true if *any* of the servers in mysql_servers has gtid_port enabled + // it is configured during commit() + // NOTE: this variable is currently NOT used, but in future will be able + // to deprecate mysql-default_session_track_gtids because proxysql will + // be automatically able to determine when to enable GTID tracking + std::atomic has_gtid_port; MySQL_HostGroups_Manager(); ~MySQL_HostGroups_Manager(); void init(); diff --git a/include/MySQL_Session.h b/include/MySQL_Session.h index 76dabf3a1..5c4764958 100644 --- a/include/MySQL_Session.h +++ b/include/MySQL_Session.h @@ -113,7 +113,6 @@ class MySQL_Session bool handler_again___verify_backend_autocommit(); bool handler_again___verify_backend_session_track_gtids(); bool handler_again___verify_backend_multi_statement(); - bool handler_again___verify_backend__generic_variable(uint32_t *be_int, char **be_var, char *def, uint32_t *fe_int, char *fe_var, enum session_status next_sess_status); bool handler_again___verify_backend_user_schema(); bool handler_again___status_SETTING_INIT_CONNECT(int *); bool handler_again___status_SETTING_LDAP_USER_VARIABLE(int *); diff --git a/include/MySQL_Thread.h b/include/MySQL_Thread.h index 422b3b23e..0699393ab 100644 --- a/include/MySQL_Thread.h +++ b/include/MySQL_Thread.h @@ -130,6 +130,7 @@ class MySQL_Thread MySQL_Connection **my_idle_conns; bool processing_idles; bool maintenance_loop; + bool retrieve_gtids_required; // if any of the servers has gtid_port enabled, this needs to be turned on too PtrArray *cached_connections; diff --git a/lib/MySQL_HostGroups_Manager.cpp b/lib/MySQL_HostGroups_Manager.cpp index 138ee313f..864522a84 100644 --- a/lib/MySQL_HostGroups_Manager.cpp +++ b/lib/MySQL_HostGroups_Manager.cpp @@ -1604,7 +1604,9 @@ bool MySQL_HostGroups_Manager::commit() { wrlock(); // purge table purge_mysql_servers_table(); - + // if any server has gtid_port enabled, use_gtid is set to true + // and then has_gtid_port is set too + bool use_gtid = false; proxy_debug(PROXY_DEBUG_MYSQL_CONNPOOL, 4, "DELETE FROM mysql_servers\n"); mydb->execute("DELETE FROM mysql_servers"); generate_mysql_servers_table(); @@ -1706,6 +1708,11 @@ bool MySQL_HostGroups_Manager::commit() { SAFE_SQLITE3_STEP2(statement1); rc=(*proxy_sqlite3_clear_bindings)(statement1); ASSERT_SQLITE_OK(rc, mydb); rc=(*proxy_sqlite3_reset)(statement1); ASSERT_SQLITE_OK(rc, mydb); + if (mysrvc->gtid_port) { + // this server has gtid_port configured, we set use_gtid + proxy_debug(PROXY_DEBUG_MYSQL_CONNPOOL, 6, "Server %u:%s:%d has gtid_port enabled, setting use_gitd=true if not already set\n", mysrvc->myhgc->hid , mysrvc->address); + use_gtid = true; + } } else { bool run_update=false; MySrvC *mysrvc=(MySrvC *)ptr; @@ -1785,11 +1792,21 @@ bool MySQL_HostGroups_Manager::commit() { rc=(*proxy_sqlite3_clear_bindings)(statement2); ASSERT_SQLITE_OK(rc, mydb); rc=(*proxy_sqlite3_reset)(statement2); ASSERT_SQLITE_OK(rc, mydb); } + if (mysrvc->gtid_port) { + // this server has gtid_port configured, we set use_gtid + proxy_debug(PROXY_DEBUG_MYSQL_CONNPOOL, 6, "Server %u:%s:%d has gtid_port enabled, setting use_gitd=true if not already set\n", mysrvc->myhgc->hid , mysrvc->address); + use_gtid = true; + } } } (*proxy_sqlite3_finalize)(statement1); (*proxy_sqlite3_finalize)(statement2); } + if (use_gtid) { + has_gtid_port = true; + } else { + has_gtid_port = false; + } if (resultset) { delete resultset; resultset=NULL; } proxy_debug(PROXY_DEBUG_MYSQL_CONNPOOL, 4, "DELETE FROM mysql_servers_incoming\n"); mydb->execute("DELETE FROM mysql_servers_incoming"); diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index 8e8fe88c9..900061c5a 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -1585,76 +1585,6 @@ void MySQL_Session::handler_again___new_thread_to_kill_connection() { // true should jump to handler_again #define NEXT_IMMEDIATE_NEW(new_st) do { set_status(new_st); return true; } while (0) -/* this function seems to be used only by - handler_again___verify_backend_session_track_gtids - Yet, the check on GTID is far from generic. - We need to deprecate this function -*/ -bool MySQL_Session::handler_again___verify_backend__generic_variable(uint32_t *be_int, char **be_var, char *def, uint32_t *fe_int, char *fe_var, enum session_status next_sess_status) { - // be_int = backend int (hash) - // be_var = backend value - // def = default - // fe_int = frontend int (has) - // fe_var = frontend value - if (*be_int == 0) { - // it is the first time we use this backend. Set value to default - if (*be_var) { - free(*be_var); - *be_var = NULL; - } - *be_var = strdup(def); - uint32_t tmp_int = SpookyHash::Hash32(*be_var, strlen(*be_var), 10); - *be_int = tmp_int; - - switch(status) { // this switch can be replaced with a simple previous_status.push(status), but it is here for readibility - case PROCESSING_QUERY: - previous_status.push(PROCESSING_QUERY); - break; - case PROCESSING_STMT_PREPARE: - previous_status.push(PROCESSING_STMT_PREPARE); - break; - case PROCESSING_STMT_EXECUTE: - previous_status.push(PROCESSING_STMT_EXECUTE); - break; - default: - assert(0); - break; - } - NEXT_IMMEDIATE_NEW(next_sess_status); - } - - if (*fe_int) { - if (*fe_int != *be_int) { - { - *be_int = *fe_int; - if (*be_var) { - free(*be_var); - *be_var = NULL; - } - if (fe_var) { - *be_var = strdup(fe_var); - } - } - switch(status) { // this switch can be replaced with a simple previous_status.push(status), but it is here for readibility - case PROCESSING_QUERY: - previous_status.push(PROCESSING_QUERY); - break; - case PROCESSING_STMT_PREPARE: - previous_status.push(PROCESSING_STMT_PREPARE); - break; - case PROCESSING_STMT_EXECUTE: - previous_status.push(PROCESSING_STMT_EXECUTE); - break; - default: - assert(0); - break; - } - NEXT_IMMEDIATE_NEW(next_sess_status); - } - } - return false; -} - bool MySQL_Session::handler_again___verify_backend_multi_statement() { if ((client_myds->myconn->options.client_flag & CLIENT_MULTI_STATEMENTS) != (mybe->server_myds->myconn->options.client_flag & CLIENT_MULTI_STATEMENTS)) { @@ -1712,15 +1642,62 @@ bool MySQL_Session::handler_again___verify_init_connect() { bool MySQL_Session::handler_again___verify_backend_session_track_gtids() { bool ret = false; proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "Session %p , client: %s , backend: %s\n", this, client_myds->myconn->options.session_track_gtids, mybe->server_myds->myconn->options.session_track_gtids); + // we first verify that the backend supports it + // if backend is old (or if it is not mysql) ignore this setting + if ((mybe->server_myds->myconn->mysql->server_capabilities & CLIENT_SESSION_TRACKING) == 0) { + // the backend doesn't support CLIENT_SESSION_TRACKING + return ret; // exit immediately + } + uint32_t b_int = mybe->server_myds->myconn->options.session_track_gtids_int; + uint32_t f_int = client_myds->myconn->options.session_track_gtids_int; + // we need to precompute and hardcode the values for OFF and OWN_GTID - ret = handler_again___verify_backend__generic_variable( - &mybe->server_myds->myconn->options.session_track_gtids_int, - &mybe->server_myds->myconn->options.session_track_gtids, - mysql_thread___default_session_track_gtids, - &client_myds->myconn->options.session_track_gtids_int, - client_myds->myconn->options.session_track_gtids, - SETTING_SESSION_TRACK_GTIDS - ); + // for performance reason we hardcoded the values + // OFF = 114160514 + if ( + (b_int == 114160514) // OFF + || + (b_int == 0) // not configured yet + ) { + if (strcmp(mysql_thread___default_session_track_gtids, (char *)"OWN_GTID")==0) { + // backend connection doesn't have session_track_gtids enabled + ret = true; + } else { + if (f_int != 0 && f_int != 114160514) { + // client wants GTID + ret = true; + } + } + } + + if (ret) { + // we deprecated handler_again___verify_backend__generic_variable + // and moved the logic here + if (mybe->server_myds->myconn->options.session_track_gtids) { // reset current value + free(mybe->server_myds->myconn->options.session_track_gtids); + mybe->server_myds->myconn->options.session_track_gtids = NULL; + } + // because the only two possible values are OWN_GTID and OFF + // and because we don't mind receiving GTIDs , if we reach here + // it means we are setting it to OWN_GTID, either because the client + // wants it, or because it is the default + // therefore we hardcode "OWN_GTID" + mybe->server_myds->myconn->options.session_track_gtids = strdup((char *)"OWN_GTID"); + mybe->server_myds->myconn->options.session_track_gtids_int = + SpookyHash::Hash32((char *)"OWN_GTID", strlen((char *)"OWN_GTID"), 10); + // we now switch status to set session_track_gtids + switch(status) { + case PROCESSING_QUERY: + case PROCESSING_STMT_PREPARE: + case PROCESSING_STMT_EXECUTE: + previous_status.push(status); + break; + default: + assert(0); + break; + } + NEXT_IMMEDIATE_NEW(SETTING_SESSION_TRACK_GTIDS); + } return ret; } @@ -5183,7 +5160,18 @@ bool MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C exit_after_SetParse = true; } else if (var == "session_track_gtids") { std::string value1 = *values; - if ((strcasecmp(value1.c_str(),"OWN_GTID")==0) || (strcasecmp(value1.c_str(),"OFF")==0)) { + if ((strcasecmp(value1.c_str(),"OWN_GTID")==0) || (strcasecmp(value1.c_str(),"OFF")==0) || (strcasecmp(value1.c_str(),"ALL_GTIDS")==0)) { + if (strcasecmp(value1.c_str(),"ALL_GTIDS")==0) { + // we convert session_track_gtids=ALL_GTIDS to session_track_gtids=OWN_GTID + std::string a = ""; + if (client_myds && client_myds->addr.addr) { + a = " . Client "; + a+= client_myds->addr.addr; + a+= ":" + std::to_string(client_myds->addr.port); + } + proxy_warning("SET session_track_gtids=ALL_GTIDS is not allowed. Switching to session_track_gtids=OWN_GTID%s\n", a.c_str()); + value1 = "OWN_GTID"; + } proxy_debug(PROXY_DEBUG_MYSQL_COM, 7, "Processing SET session_track_gtids value %s\n", value1.c_str()); uint32_t session_track_gtids_int=SpookyHash::Hash32(value1.c_str(),value1.length(),10); if (client_myds->myconn->options.session_track_gtids_int != session_track_gtids_int) { diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index 063da4c98..db10dd809 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -4151,6 +4151,8 @@ __run_skip_1a: maintenance_loop=true; servers_table_version_previous = servers_table_version_current; servers_table_version_current = MyHGM->get_servers_table_version(); + // during a maintenance loop (every 1 second) we read has_gtid_port from MyHGM + retrieve_gtids_required = MyHGM->has_gtid_port; } else { maintenance_loop=false; } @@ -4944,6 +4946,7 @@ MySQL_Thread::MySQL_Thread() { last_maintenance_time=0; maintenance_loop=true; + retrieve_gtids_required = false; servers_table_version_previous=0; servers_table_version_current=0;