From babac8ec6fa4fadecd47f968aae29ebc7fa003e6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 22 Mar 2026 17:09:24 +0000 Subject: [PATCH 1/4] Fix compilation error: add missing #include in MonitorHealthDecision.cpp The int64_t cast in can_unshun_server() requires , which was not included, causing build failure. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/MonitorHealthDecision.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/MonitorHealthDecision.cpp b/lib/MonitorHealthDecision.cpp index 5b37cb03be..83c1dbd5c0 100644 --- a/lib/MonitorHealthDecision.cpp +++ b/lib/MonitorHealthDecision.cpp @@ -11,6 +11,7 @@ */ #include "MonitorHealthDecision.h" +#include bool should_shun_on_connect_errors( unsigned int errors_this_second, From f88e3af4ae962bd9f4e1f11db0e09f9fd355ff0e Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Mon, 23 Mar 2026 11:19:26 +0000 Subject: [PATCH 2/4] Extract hostgroup routing and locking logic from MySQL and PgSQL sessions This commit completes Milestone 3 Phase 3.5 by moving hostgroup routing and locking decisions out of the MySQL_Session and PgSQL_Session classes. The logic is now encapsulated in standalone, pure functions designed for easier testing and better maintainability. Key changes: - Created include/MySQL_HostGroup_Routing.h and lib/MySQL_HostGroup_Routing.cpp: - Implements resolve_hostgroup_routing(), a pure function that handles: - Mirroring logic (highest priority). - Intercepting SHOW WARNINGS and routing to the previous HG. - Routing SELECT LAST_INSERT_ID() / @@IDENTITY queries to the HG where the last affected rows occurred. - Applying destination_hostgroup from Query Processor Output (QPO). - Enforcing hostgroup locking (mysql-set_query_lock_on_hostgroup). - Created include/PgSQL_HostGroup_Routing.h and lib/PgSQL_HostGroup_Routing.cpp: - Implements resolve_pgsql_hostgroup_routing() with similar logic adapted for PostgreSQL sessions. - Refactored lib/MySQL_Session.cpp and lib/PgSQL_Session.cpp: - Replaced scattered inline logic with calls to the new routing functions. - Integration points include STATE_SLEEP, STMT_PREPARE, STMT_EXECUTE, and the main query execution path. - Enhanced Unit Testing: - Added test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp (15 test cases). - Added test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp (10 test cases). - Verified all 25 test cases pass successfully. - Updated Build System: - Modified lib/Makefile to include the new object files. - Updated test/tap/tests/unit/Makefile to include unit test targets. This refactoring significantly reduces coupling between the session state and routing logic, providing a foundation for more robust session orchestration and simplified unit testing of critical ProxySQL features. Issue: #5493 --- include/MySQL_HostGroup_Routing.h | 62 ++++ include/PgSQL_HostGroup_Routing.h | 54 +++ lib/Makefile | 2 +- lib/MySQL_HostGroup_Routing.cpp | 76 ++++ lib/MySQL_Session.cpp | 326 ++++++++++++------ lib/PgSQL_HostGroup_Routing.cpp | 52 +++ lib/PgSQL_Session.cpp | 252 ++++++++------ test/tap/tests/unit/Makefile | 8 +- .../tests/unit/MySQL_HostGroup_Routing-t.cpp | 126 +++++++ .../tests/unit/PgSQL_HostGroup_Routing-t.cpp | 92 +++++ 10 files changed, 836 insertions(+), 214 deletions(-) create mode 100644 include/MySQL_HostGroup_Routing.h create mode 100644 include/PgSQL_HostGroup_Routing.h create mode 100644 lib/MySQL_HostGroup_Routing.cpp create mode 100644 lib/PgSQL_HostGroup_Routing.cpp create mode 100644 test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp create mode 100644 test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp diff --git a/include/MySQL_HostGroup_Routing.h b/include/MySQL_HostGroup_Routing.h new file mode 100644 index 0000000000..884bb5992e --- /dev/null +++ b/include/MySQL_HostGroup_Routing.h @@ -0,0 +1,62 @@ +#ifndef __MYSQL_HOSTGROUP_ROUTING_H +#define __MYSQL_HOSTGROUP_ROUTING_H + +#include + +/** + * @struct MySQL_Routing_Session_State + * @brief Represents the session state relevant for hostgroup routing decisions. + */ +struct MySQL_Routing_Session_State { + int current_hostgroup; + int default_hostgroup; + int locked_on_hostgroup; + int transaction_persistent_hostgroup; + int last_HG_affected_rows; + int warning_in_hg; + bool autocommit; + int autocommit_on_hostgroup; + bool mirror; +}; + +/** + * @struct MySQL_Routing_QPO_State + * @brief Represents the Query Processor Output relevant for hostgroup routing decisions. + */ +struct MySQL_Routing_QPO_State { + int destination_hostgroup; + bool is_set_statement; // Derived from query parsing + bool is_show_warnings; // Derived from query parsing + bool is_last_insert_id; // Derived from query parsing + bool is_version_query; // Derived from query parsing +}; + +/** + * @struct MySQL_Routing_Result + * @brief Represents the output of the hostgroup routing decision. + */ +struct MySQL_Routing_Result { + int new_current_hostgroup; + int new_locked_on_hostgroup; + bool lock_hostgroup; + bool error; + std::string error_msg; +}; + +/** + * @brief Resolves the target hostgroup and locking decisions based on session and QPO state. + * + * This is a pure function designed to be easily testable. + * + * @param sess_state Current session state. + * @param qpo_state Query Processor Output state. + * @param set_query_lock_on_hostgroup Global configuration (mysql-set_query_lock_on_hostgroup). + * @return MySQL_Routing_Result The routing decision. + */ +MySQL_Routing_Result resolve_hostgroup_routing( + const MySQL_Routing_Session_State& sess_state, + const MySQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup +); + +#endif // __MYSQL_HOSTGROUP_ROUTING_H diff --git a/include/PgSQL_HostGroup_Routing.h b/include/PgSQL_HostGroup_Routing.h new file mode 100644 index 0000000000..9cc23777fb --- /dev/null +++ b/include/PgSQL_HostGroup_Routing.h @@ -0,0 +1,54 @@ +#ifndef __PGSQL_HOSTGROUP_ROUTING_H +#define __PGSQL_HOSTGROUP_ROUTING_H + +#include + +/** + * @struct PgSQL_Routing_Session_State + * @brief Represents the session state relevant for hostgroup routing decisions in PostgreSQL. + */ +struct PgSQL_Routing_Session_State { + int current_hostgroup; + int default_hostgroup; + int locked_on_hostgroup; + int transaction_persistent_hostgroup; +}; + +/** + * @struct PgSQL_Routing_QPO_State + * @brief Represents the Query Processor Output relevant for hostgroup routing decisions in PostgreSQL. + */ +struct PgSQL_Routing_QPO_State { + int destination_hostgroup; + bool lock_hostgroup; // Derived from query parsing +}; + +/** + * @struct PgSQL_Routing_Result + * @brief Represents the output of the hostgroup routing decision for PostgreSQL. + */ +struct PgSQL_Routing_Result { + int new_current_hostgroup; + int new_locked_on_hostgroup; + bool lock_hostgroup; + bool error; + std::string error_msg; +}; + +/** + * @brief Resolves the target hostgroup and locking decisions based on session and QPO state. + * + * This is a pure function designed to be easily testable. + * + * @param sess_state Current session state. + * @param qpo_state Query Processor Output state. + * @param set_query_lock_on_hostgroup Global configuration (pgsql-set_query_lock_on_hostgroup). + * @return PgSQL_Routing_Result The routing decision. + */ +PgSQL_Routing_Result resolve_pgsql_hostgroup_routing( + const PgSQL_Routing_Session_State& sess_state, + const PgSQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup +); + +#endif // __PGSQL_HOSTGROUP_ROUTING_H diff --git a/lib/Makefile b/lib/Makefile index f7f24075c0..4c4f7b7564 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -88,7 +88,7 @@ MYCXXFLAGS := $(STDCPP) $(MYCFLAGS) $(PSQLCH) $(PSQLGA) $(PSQL31) $(PSQLFFTO) $( default: libproxysql.a .PHONY: default -_OBJ_CXX := ProxySQL_GloVars.oo network.oo debug.oo configfile.oo Query_Cache.oo SpookyV2.oo MySQL_Authentication.oo gen_utils.oo sqlite3db.oo mysql_connection.oo MySQL_HostGroups_Manager.oo mysql_data_stream.oo MySQL_Thread.oo MySQL_Session.oo MySQL_Protocol.oo mysql_backend.oo Query_Processor.oo MySQL_Query_Processor.oo PgSQL_Query_Processor.oo ProxySQL_Admin.oo ProxySQL_Config.oo ProxySQL_Restapi.oo MySQL_Monitor.oo MySQL_Logger.oo log_utils.oo thread.oo MySQL_PreparedStatement.oo ProxySQL_Cluster.oo ClickHouse_Authentication.oo ClickHouse_Server.oo ProxySQL_Statistics.oo Chart_bundle_js.oo ProxySQL_HTTP_Server.oo ProxySQL_RESTAPI_Server.oo font-awesome.min.css.oo main-bundle.min.css.oo MySQL_Variables.oo c_tokenizer.oo proxysql_utils.oo proxysql_coredump.oo proxysql_sslkeylog.oo \ +_OBJ_CXX := ProxySQL_GloVars.oo network.oo debug.oo configfile.oo Query_Cache.oo SpookyV2.oo MySQL_Authentication.oo gen_utils.oo sqlite3db.oo mysql_connection.oo MySQL_HostGroups_Manager.oo mysql_data_stream.oo MySQL_Thread.oo MySQL_Session.oo MySQL_HostGroup_Routing.oo PgSQL_Session.oo PgSQL_HostGroup_Routing.oo MySQL_Protocol.oo mysql_backend.oo Query_Processor.oo MySQL_Query_Processor.oo PgSQL_Query_Processor.oo ProxySQL_Admin.oo ProxySQL_Config.oo ProxySQL_Restapi.oo MySQL_Monitor.oo MySQL_Logger.oo log_utils.oo thread.oo MySQL_PreparedStatement.oo ProxySQL_Cluster.oo ClickHouse_Authentication.oo ClickHouse_Server.oo ProxySQL_Statistics.oo Chart_bundle_js.oo ProxySQL_HTTP_Server.oo ProxySQL_RESTAPI_Server.oo font-awesome.min.css.oo main-bundle.min.css.oo MySQL_Variables.oo c_tokenizer.oo proxysql_utils.oo proxysql_coredump.oo proxysql_sslkeylog.oo \ sha256crypt.oo \ BaseSrvList.oo BaseHGC.oo Base_HostGroups_Manager.oo \ QP_rule_text.oo QP_query_digest_stats.oo \ diff --git a/lib/MySQL_HostGroup_Routing.cpp b/lib/MySQL_HostGroup_Routing.cpp new file mode 100644 index 0000000000..3e1e98ffca --- /dev/null +++ b/lib/MySQL_HostGroup_Routing.cpp @@ -0,0 +1,76 @@ +#include "MySQL_HostGroup_Routing.h" + +MySQL_Routing_Result resolve_hostgroup_routing( + const MySQL_Routing_Session_State& sess_state, + const MySQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup +) { + MySQL_Routing_Result result; + result.new_current_hostgroup = sess_state.current_hostgroup; + result.new_locked_on_hostgroup = sess_state.locked_on_hostgroup; + result.lock_hostgroup = false; + result.error = false; + result.error_msg = ""; + + // 1. Mirroring + if (sess_state.mirror) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + return result; + } + + // 2. SHOW WARNINGS / SHOW COUNT(*) WARNINGS + if (qpo_state.is_show_warnings) { + if (sess_state.warning_in_hg > -1) { + result.new_current_hostgroup = sess_state.warning_in_hg; + } + return result; + } + + // 3. LAST_INSERT_ID / @@IDENTITY + if (qpo_state.is_last_insert_id) { + if (sess_state.last_HG_affected_rows >= 0) { + result.new_current_hostgroup = sess_state.last_HG_affected_rows; + return result; + } + } + + // 4. Default routing from QPO + if (qpo_state.destination_hostgroup >= 0) { + if (sess_state.transaction_persistent_hostgroup == -1) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + } + } + + // 5. Hostgroup Locking Decisions (mysql-set_query_lock_on_hostgroup) + if (set_query_lock_on_hostgroup == 1) { + // Algorithm introduced in ProxySQL 2.0.6 + if (result.new_locked_on_hostgroup < 0) { + if (qpo_state.is_set_statement) { + // If it's a SET statement that caused locking (determined by parser) + // In a pure function, we assume qpo_state.is_set_statement implies it should lock + result.lock_hostgroup = true; + result.new_locked_on_hostgroup = result.new_current_hostgroup; + } + } + + if (result.new_locked_on_hostgroup >= 0) { + if (result.new_current_hostgroup != result.new_locked_on_hostgroup) { + result.error = true; + result.error_msg = "ProxySQL Error: connection is locked to hostgroup " + + std::to_string(result.new_locked_on_hostgroup) + + " but trying to reach hostgroup " + + std::to_string(result.new_current_hostgroup); + return result; + } + } + } else { + // Legacy behavior before 2.0.6 + if (sess_state.transaction_persistent_hostgroup == -1) { + if (qpo_state.destination_hostgroup < 0) { + result.new_current_hostgroup = sess_state.default_hostgroup; + } + } + } + + return result; +} diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index 5946e95a5c..2c9edf2399 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -23,6 +23,7 @@ using json = nlohmann::json; #include "MySQL_Authentication.hpp" #include "MySQL_LDAP_Authentication.hpp" #include "MySQL_Protocol.h" +#include "MySQL_HostGroup_Routing.h" #include "SQLite3_Server.h" #include "MySQL_Variables.h" #include "ProxySQL_Cluster.hpp" @@ -3328,33 +3329,60 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C if (rc_break==true) { return; } - if (mysql_thread___set_query_lock_on_hostgroup == 1) { // algorithm introduced in 2.0.6 - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - locked_on_hostgroup = current_hostgroup; - } + + { + MySQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.warning_in_hg = warning_in_hg; + sess_state.autocommit = autocommit; + sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; + sess_state.mirror = mirror; + + MySQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = qpo->destination_hostgroup; + qpo_state.is_set_statement = lock_hostgroup; + if (CurrentQuery.QueryParserArgs.digest_text) { + const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; + const size_t dig_len = strlen(dig_text); + if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + qpo_state.is_show_warnings = true; + } + if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { + qpo_state.is_last_insert_id = true; + } + } + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess_state, qpo_state, mysql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + client_myds->DSS=STATE_QUERY_SENT_NET; + int l = CurrentQuery.QueryLength; + char *end = (char *)""; + if (l>256) { + l=253; + end = (char *)"..."; + } + string nqn = string((char *)CurrentQuery.QueryPointer,l); + char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); + sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); + thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; + RequestEnd(NULL, 9005, buf); + free(buf); + l_free(pkt.size,pkt.ptr); + return; } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - client_myds->DSS=STATE_QUERY_SENT_NET; - int l = CurrentQuery.QueryLength; - char *end = (char *)""; - if (l>256) { - l=253; - end = (char *)"..."; - } - string nqn = string((char *)CurrentQuery.QueryPointer,l); - char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s"; - char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); - sprintf(buf, err_msg, current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); - client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); - thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; - RequestEnd(NULL, 9005, buf); - free(buf); - l_free(pkt.size,pkt.ptr); - return; - } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; } } mybe=find_or_create_backend(current_hostgroup); @@ -3500,34 +3528,65 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C if (rc_break==true) { return; } - if (mysql_thread___set_query_lock_on_hostgroup == 1) { // algorithm introduced in 2.0.6 - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - locked_on_hostgroup = current_hostgroup; - } + + { + MySQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.warning_in_hg = warning_in_hg; + sess_state.autocommit = autocommit; + sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; + sess_state.mirror = mirror; + + MySQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = qpo->destination_hostgroup; + qpo_state.is_set_statement = lock_hostgroup; + if (CurrentQuery.QueryParserArgs.digest_text) { + const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; + const size_t dig_len = strlen(dig_text); + if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + qpo_state.is_show_warnings = true; + } + if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { + qpo_state.is_last_insert_id = true; + } + } else if (CurrentQuery.stmt_info && CurrentQuery.stmt_info->query) { + const char* query_text = CurrentQuery.stmt_info->query; + if (strcasestr(query_text,"LAST_INSERT_ID") || strcasestr(query_text,"@@IDENTITY")) { + qpo_state.is_last_insert_id = true; + } + } + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess_state, qpo_state, mysql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + client_myds->DSS=STATE_QUERY_SENT_NET; + int l = CurrentQuery.stmt_info->query_length; + char *end = (char *)""; + if (l>256) { + l=253; + end = (char *)"..."; + } + string nqn = string((char *)CurrentQuery.stmt_info->query,l); + char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); + sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); + thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; + RequestEnd(NULL, 9005, buf); + free(buf); + l_free(pkt.size,pkt.ptr); + return; } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - client_myds->DSS=STATE_QUERY_SENT_NET; - //int l = CurrentQuery.QueryLength; - int l = CurrentQuery.stmt_info->query_length; - char *end = (char *)""; - if (l>256) { - l=253; - end = (char *)"..."; - } - string nqn = string((char *)CurrentQuery.stmt_info->query,l); - char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s"; - char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); - sprintf(buf, err_msg, current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); - client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); - thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; - RequestEnd(NULL, 9005, buf); - free(buf); - l_free(pkt.size,pkt.ptr); - return; - } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; } } mybe=find_or_create_backend(current_hostgroup); @@ -5181,15 +5240,18 @@ int MySQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { case STATE_SLEEP: // only this section can be executed ALSO by mirror command_counters->incr(thread->curtime/1000000); if (transaction_persistent_hostgroup==-1) { - if (mysql_thread___set_query_lock_on_hostgroup == 0) { // behavior before 2.0.6 - current_hostgroup=default_hostgroup; - } else { - if (locked_on_hostgroup==-1) { - current_hostgroup = default_hostgroup; - } else { - current_hostgroup = locked_on_hostgroup; - } - } + MySQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + sess_state.mirror = mirror; + + MySQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = -1; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess_state, qpo_state, mysql_thread___set_query_lock_on_hostgroup); + current_hostgroup = res.new_current_hostgroup; } proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "Session=%p , client_myds=%p . Statuses: WAITING_CLIENT_DATA - STATE_SLEEP\n", this, client_myds); @@ -5339,40 +5401,60 @@ int MySQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { if (autocommit_on_hostgroup>=0) { } - if (mysql_thread___set_query_lock_on_hostgroup == 1) { // algorithm introduced in 2.0.6 - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - if ( qpo->destination_hostgroup >= 0 ) { - if (transaction_persistent_hostgroup == -1) { - current_hostgroup=qpo->destination_hostgroup; - } - } - locked_on_hostgroup = current_hostgroup; - thread->status_variables.stvar[st_var_hostgroup_locked]++; - thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; + + { + MySQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.warning_in_hg = warning_in_hg; + sess_state.autocommit = autocommit; + sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; + sess_state.mirror = mirror; + + MySQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = qpo->destination_hostgroup; + qpo_state.is_set_statement = lock_hostgroup; + if (CurrentQuery.QueryParserArgs.digest_text) { + const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; + const size_t dig_len = strlen(dig_text); + if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + qpo_state.is_show_warnings = true; + } + if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { + qpo_state.is_last_insert_id = true; } } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - client_myds->DSS=STATE_QUERY_SENT_NET; - int l = CurrentQuery.QueryLength; - char *end = (char *)""; - if (l>256) { - l=253; - end = (char *)"..."; - } - string nqn = string((char *)CurrentQuery.QueryPointer,l); - char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s"; - char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); - sprintf(buf, err_msg, current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); - client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); - thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; - RequestEnd(NULL, 9005, buf); - free(buf); - l_free(pkt.size,pkt.ptr); - break; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess_state, qpo_state, mysql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + client_myds->DSS=STATE_QUERY_SENT_NET; + int l = CurrentQuery.QueryLength; + char *end = (char *)""; + if (l>256) { + l=253; + end = (char *)"..."; } + string nqn = string((char *)CurrentQuery.QueryPointer,l); + char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); + sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); + thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; + RequestEnd(NULL, 9005, buf); + free(buf); + l_free(pkt.size,pkt.ptr); + break; + } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; } } mybe=find_or_create_backend(current_hostgroup); @@ -8262,26 +8344,52 @@ bool MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C if ( qpo->next_query_flagIN >= 0 ) { next_query_flagIN=qpo->next_query_flagIN; } - if ( qpo->destination_hostgroup >= 0 ) { - if (transaction_persistent_hostgroup == -1) { - current_hostgroup=qpo->destination_hostgroup; - } - } - if (mysql_thread___set_query_lock_on_hostgroup == 1) { // algorithm introduced in 2.0.6 - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - client_myds->DSS=STATE_QUERY_SENT_NET; - char buf[140]; - sprintf(buf,"ProxySQL Error: connection is locked to hostgroup %d but trying to reach hostgroup %d", locked_on_hostgroup, current_hostgroup); - client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9006,(char *)"Y0000",buf); - thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; - RequestEnd(NULL, 9006, buf); - l_free(pkt->size,pkt->ptr); - return true; + { + MySQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.warning_in_hg = warning_in_hg; + sess_state.autocommit = autocommit; + sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; + sess_state.mirror = mirror; + + MySQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = qpo->destination_hostgroup; + qpo_state.is_set_statement = *lock_hostgroup; + if (CurrentQuery.QueryParserArgs.digest_text) { + const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; + const size_t dig_len = strlen(dig_text); + if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + qpo_state.is_show_warnings = true; } + if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { + qpo_state.is_last_insert_id = true; + } + } + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess_state, qpo_state, mysql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + client_myds->DSS=STATE_QUERY_SENT_NET; + client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9006,(char *)"Y0000", (char*)res.error_msg.c_str()); + thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; + RequestEnd(NULL, 9006, (char*)res.error_msg.c_str()); + l_free(pkt->size,pkt->ptr); + return true; + } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; } } + return false; } diff --git a/lib/PgSQL_HostGroup_Routing.cpp b/lib/PgSQL_HostGroup_Routing.cpp new file mode 100644 index 0000000000..bf14017b27 --- /dev/null +++ b/lib/PgSQL_HostGroup_Routing.cpp @@ -0,0 +1,52 @@ +#include "PgSQL_HostGroup_Routing.h" + +PgSQL_Routing_Result resolve_pgsql_hostgroup_routing( + const PgSQL_Routing_Session_State& sess_state, + const PgSQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup +) { + PgSQL_Routing_Result result; + result.new_current_hostgroup = sess_state.current_hostgroup; + result.new_locked_on_hostgroup = sess_state.locked_on_hostgroup; + result.lock_hostgroup = false; + result.error = false; + result.error_msg = ""; + + // Default routing from QPO + if (qpo_state.destination_hostgroup >= 0) { + if (sess_state.transaction_persistent_hostgroup == -1) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + } + } + + // Hostgroup Locking Decisions (pgsql-set_query_lock_on_hostgroup) + if (set_query_lock_on_hostgroup == 1) { + // Algorithm introduced in ProxySQL 2.0.6 + if (result.new_locked_on_hostgroup < 0) { + if (qpo_state.lock_hostgroup) { + result.lock_hostgroup = true; + result.new_locked_on_hostgroup = result.new_current_hostgroup; + } + } + + if (result.new_locked_on_hostgroup >= 0) { + if (result.new_current_hostgroup != result.new_locked_on_hostgroup) { + result.error = true; + result.error_msg = "ProxySQL Error: connection is locked to hostgroup " + + std::to_string(result.new_locked_on_hostgroup) + + " but trying to reach hostgroup " + + std::to_string(result.new_current_hostgroup); + return result; + } + } + } else { + // Legacy behavior before 2.0.6 + if (sess_state.transaction_persistent_hostgroup == -1) { + if (qpo_state.destination_hostgroup < 0) { + result.new_current_hostgroup = sess_state.default_hostgroup; + } + } + } + + return result; +} diff --git a/lib/PgSQL_Session.cpp b/lib/PgSQL_Session.cpp index 6b7f05fd16..f80380bdd7 100644 --- a/lib/PgSQL_Session.cpp +++ b/lib/PgSQL_Session.cpp @@ -15,6 +15,7 @@ using json = nlohmann::json; #include "MySQL_Data_Stream.h" #include "PgSQL_Query_Processor.h" #include "PgSQL_PreparedStatement.h" +#include "PgSQL_HostGroup_Routing.h" #include "PgSQL_Logger.hpp" #include "StatCounters.h" #include "PgSQL_Authentication.h" @@ -1974,15 +1975,17 @@ int PgSQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { case STATE_SLEEP: // only this section can be executed ALSO by mirror command_counters->incr(thread->curtime / 1000000); if (transaction_persistent_hostgroup == -1) { - if (pgsql_thread___set_query_lock_on_hostgroup == 0) { // behavior before 2.0.6 - current_hostgroup = default_hostgroup; - } else { - if (locked_on_hostgroup == -1) { - current_hostgroup = default_hostgroup; - } else { - current_hostgroup = locked_on_hostgroup; - } - } + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = -1; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + current_hostgroup = res.new_current_hostgroup; } proxy_debug(PROXY_DEBUG_MYSQL_CONNECTION, 5, "Session=%p , client_myds=%p . Statuses: WAITING_CLIENT_DATA - STATE_SLEEP\n", this, client_myds); if (session_fast_forward) { // if it is fast forward @@ -2151,41 +2154,45 @@ int PgSQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { //handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY___create_mirror_session(); } - if (pgsql_thread___set_query_lock_on_hostgroup == 1) { // algorithm introduced in 2.0.6 - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - if (qpo->destination_hostgroup >= 0) { - if (transaction_persistent_hostgroup == -1) { - current_hostgroup = qpo->destination_hostgroup; - } - } - locked_on_hostgroup = current_hostgroup; - thread->status_variables.stvar[st_var_hostgroup_locked]++; - thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; + { + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = qpo->destination_hostgroup; + qpo_state.lock_hostgroup = lock_hostgroup; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + client_myds->DSS = STATE_QUERY_SENT_NET; + int l = CurrentQuery.QueryLength; + char* end = (char*)""; + if (l > 256) { + l = 253; + end = (char*)"..."; } + string nqn = string((char*)CurrentQuery.QueryPointer, l); + const char* err_msg = "Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + char* buf = (char*)malloc(strlen(err_msg) + strlen(nqn.c_str()) + strlen(end) + 64); + sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + client_myds->myprot.generate_error_packet(true, true, buf, PGSQL_ERROR_CODES::ERRCODE_RAISE_EXCEPTION, + false, true); + thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; + RequestEnd(NULL, true); + free(buf); + l_free(pkt.size, pkt.ptr); + break; } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - client_myds->DSS = STATE_QUERY_SENT_NET; - int l = CurrentQuery.QueryLength; - char* end = (char*)""; - if (l > 256) { - l = 253; - end = (char*)"..."; - } - string nqn = string((char*)CurrentQuery.QueryPointer, l); - const char* err_msg = "Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; - char* buf = (char*)malloc(strlen(err_msg) + strlen(nqn.c_str()) + strlen(end) + 64); - sprintf(buf, err_msg, current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); - client_myds->myprot.generate_error_packet(true, true, buf, PGSQL_ERROR_CODES::ERRCODE_RAISE_EXCEPTION, - false, true); - thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; - RequestEnd(NULL, true); - free(buf); - l_free(pkt.size, pkt.ptr); - break; - } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; } } mybe = find_or_create_backend(current_hostgroup); @@ -4717,24 +4724,31 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___PGSQL_Q next_query_flagIN = qpo->next_query_flagIN; } - if (qpo->destination_hostgroup >= 0 && transaction_persistent_hostgroup == -1) { - current_hostgroup = qpo->destination_hostgroup; - } + { + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = qpo->destination_hostgroup; + qpo_state.lock_hostgroup = false; // Not set in this state - // Hostgroup locking check - if (pgsql_thread___set_query_lock_on_hostgroup == 1 && locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + + if (res.error) { client_myds->DSS = STATE_QUERY_SENT_NET; - char buf[140]; - sprintf(buf, "ProxySQL Error: connection is locked to hostgroup %d but trying to reach hostgroup %d", - locked_on_hostgroup, current_hostgroup); - client_myds->myprot.generate_error_packet(true, true, buf, + client_myds->myprot.generate_error_packet(true, true, (char*)res.error_msg.c_str(), PGSQL_ERROR_CODES::ERRCODE_RAISE_EXCEPTION, false); thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; RequestEnd(NULL, true); l_free(pkt->size, pkt->ptr); return true; } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; } return false; @@ -6109,19 +6123,30 @@ int PgSQL_Session::handle_post_sync_parse_message(PgSQL_Parse_Message* parse_msg this, client_myds, previous_hostgroup); } - if (pgsql_thread___set_query_lock_on_hostgroup == 1) { - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - locked_on_hostgroup = current_hostgroup; - } + { + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = -1; // Not relevant here as current_hostgroup was already reset to previous + qpo_state.lock_hostgroup = lock_hostgroup; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + handle_post_sync_locked_on_hostgroup_error((const char*)CurrentQuery.QueryPointer, CurrentQuery.QueryLength); + l_free(parse_pkt.size, parse_pkt.ptr); + return 2; } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - handle_post_sync_locked_on_hostgroup_error((const char*)CurrentQuery.QueryPointer, CurrentQuery.QueryLength); - l_free(parse_pkt.size, parse_pkt.ptr); - return 2; - } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + // PgSQL session doesn't seem to update status variables here like MySQL does, + // but we keep the logic consistent. } } @@ -6355,20 +6380,27 @@ int PgSQL_Session::handle_post_sync_describe_message(PgSQL_Describe_Message* des proxy_debug(PROXY_DEBUG_MYSQL_COM, 5, "Session=%p client_myds=%p. Using previous hostgroup '%d'\n", this, client_myds, previous_hostgroup); } - if (pgsql_thread___set_query_lock_on_hostgroup == 1) { - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - locked_on_hostgroup = current_hostgroup; - } - } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - handle_post_sync_locked_on_hostgroup_error(CurrentQuery.extended_query_info.stmt_info->query, - CurrentQuery.extended_query_info.stmt_info->query_length); - return 2; - } + { + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = -1; // Not relevant here as current_hostgroup was already reset to previous + qpo_state.lock_hostgroup = lock_hostgroup; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + handle_post_sync_locked_on_hostgroup_error(CurrentQuery.extended_query_info.stmt_info->query, + CurrentQuery.extended_query_info.stmt_info->query_length); + return 2; } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; } if (extended_query_frame.empty() == true) { @@ -6501,20 +6533,27 @@ int PgSQL_Session::handle_post_sync_bind_message(PgSQL_Bind_Message* bind_msg) { this, client_myds, previous_hostgroup); } - if (pgsql_thread___set_query_lock_on_hostgroup == 1) { - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - locked_on_hostgroup = current_hostgroup; - } - } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - handle_post_sync_locked_on_hostgroup_error(CurrentQuery.extended_query_info.stmt_info->query, - CurrentQuery.extended_query_info.stmt_info->query_length); - return 2; - } + { + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = -1; // Not relevant here as current_hostgroup was already reset to previous + qpo_state.lock_hostgroup = lock_hostgroup; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + handle_post_sync_locked_on_hostgroup_error(CurrentQuery.extended_query_info.stmt_info->query, + CurrentQuery.extended_query_info.stmt_info->query_length); + return 2; } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; } bind_waiting_for_execute.reset(bind_msg->release()); // release the ownership of the bind message @@ -6642,20 +6681,27 @@ int PgSQL_Session::handle_post_sync_execute_message(PgSQL_Execute_Message* execu this, client_myds, previous_hostgroup); } - if (pgsql_thread___set_query_lock_on_hostgroup == 1) { - if (locked_on_hostgroup < 0) { - if (lock_hostgroup) { - // we are locking on hostgroup now - locked_on_hostgroup = current_hostgroup; - } - } - if (locked_on_hostgroup >= 0) { - if (current_hostgroup != locked_on_hostgroup) { - handle_post_sync_locked_on_hostgroup_error(CurrentQuery.extended_query_info.stmt_info->query, - CurrentQuery.extended_query_info.stmt_info->query_length); - return 2; - } + { + PgSQL_Routing_Session_State sess_state = {0}; + sess_state.current_hostgroup = current_hostgroup; + sess_state.default_hostgroup = default_hostgroup; + sess_state.locked_on_hostgroup = locked_on_hostgroup; + sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; + + PgSQL_Routing_QPO_State qpo_state = {0}; + qpo_state.destination_hostgroup = -1; // Not relevant here as current_hostgroup was already reset to previous + qpo_state.lock_hostgroup = lock_hostgroup; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess_state, qpo_state, pgsql_thread___set_query_lock_on_hostgroup); + + if (res.error) { + handle_post_sync_locked_on_hostgroup_error(CurrentQuery.extended_query_info.stmt_info->query, + CurrentQuery.extended_query_info.stmt_info->query_length); + return 2; } + + current_hostgroup = res.new_current_hostgroup; + locked_on_hostgroup = res.new_locked_on_hostgroup; } if (extended_query_frame.empty() == true) { diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index 0d56f1758e..80fe97f141 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -231,7 +231,7 @@ $(ODIR)/test_init.o: $(TEST_HELPERS_DIR)/test_init.cpp | $(ODIR) # Unit test targets # =========================================================================== -UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t protocol_unit-t auth_unit-t connection_pool_unit-t rule_matching_unit-t hostgroups_unit-t monitor_health_unit-t +UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t protocol_unit-t auth_unit-t connection_pool_unit-t rule_matching_unit-t hostgroups_unit-t monitor_health_unit-t MySQL_HostGroup_Routing-t PgSQL_HostGroup_Routing-t .PHONY: all all: $(UNIT_TESTS) @@ -290,6 +290,12 @@ monitor_health_unit-t: monitor_health_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYS $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ $(ALLOW_MULTI_DEF) -o $@ +MySQL_HostGroup_Routing-t: MySQL_HostGroup_Routing-t.cpp $(PROXYSQL_PATH)/lib/MySQL_HostGroup_Routing.cpp $(ODIR)/tap.o + $(CXX) $^ $(IDIRS) $(OPT) -o $@ + +PgSQL_HostGroup_Routing-t: PgSQL_HostGroup_Routing-t.cpp $(PROXYSQL_PATH)/lib/PgSQL_HostGroup_Routing.cpp $(ODIR)/tap.o + $(CXX) $^ $(IDIRS) $(OPT) -o $@ + # =========================================================================== # Clean diff --git a/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp b/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp new file mode 100644 index 0000000000..9b945e739d --- /dev/null +++ b/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp @@ -0,0 +1,126 @@ +#include +#include +#include "tap.h" +#include "MySQL_HostGroup_Routing.h" + +// TAP noise tool stubs +extern "C" int get_noise_tools_count() { return 0; } +extern "C" void stop_noise_tools() {} +std::mutex noise_failure_mutex; +std::vector noise_failures; + +void test_mirroring() { + MySQL_Routing_Session_State sess = {0}; + sess.mirror = true; + sess.current_hostgroup = 10; + + MySQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = 20; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 20, "Mirroring: current_hostgroup should be destination_hostgroup from QPO"); + ok(res.error == false, "Mirroring: no error expected"); +} + +void test_show_warnings() { + MySQL_Routing_Session_State sess = {0}; + sess.warning_in_hg = 15; + sess.current_hostgroup = 10; + + MySQL_Routing_QPO_State qpo = {0}; + qpo.is_show_warnings = true; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 15, "SHOW WARNINGS: current_hostgroup should be warning_in_hg"); + + sess.warning_in_hg = -1; + res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 10, "SHOW WARNINGS: current_hostgroup should remain unchanged if warning_in_hg is -1"); +} + +void test_last_insert_id() { + MySQL_Routing_Session_State sess = {0}; + sess.last_HG_affected_rows = 25; + sess.current_hostgroup = 10; + + MySQL_Routing_QPO_State qpo = {0}; + qpo.is_last_insert_id = true; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 25, "LAST_INSERT_ID: current_hostgroup should be last_HG_affected_rows"); +} + +void test_locking_success() { + MySQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 10; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = -1; + sess.transaction_persistent_hostgroup = -1; + + MySQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = 20; + qpo.is_set_statement = true; + + // Test initial locking + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 20, "Locking: current_hostgroup updated to destination (expected 20, got %d)", res.new_current_hostgroup); + ok(res.lock_hostgroup == true, "Locking: lock_hostgroup flag set"); + ok(res.new_locked_on_hostgroup == 20, "Locking: new_locked_on_hostgroup set to 20 (got %d)", res.new_locked_on_hostgroup); + ok(res.error == false, "Locking: no error expected"); + + // Test subsequent query on same hostgroup + sess.current_hostgroup = 20; + sess.locked_on_hostgroup = 20; + qpo.is_set_statement = false; + qpo.destination_hostgroup = 20; + res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 20, "Locked: current_hostgroup remains 20"); + ok(res.error == false, "Locked: no error when hostgroup matches"); +} + +void test_locking_error() { + MySQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 20; + sess.locked_on_hostgroup = 20; + sess.transaction_persistent_hostgroup = -1; + + MySQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = 30; // Trying to reach a different hostgroup + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.error == true, "Locked Error: error set when trying to reach different hostgroup"); + ok(res.error_msg.find("locked to hostgroup 20") != std::string::npos, "Locked Error: error message contains correct hostgroup"); +} + +void test_legacy_behavior() { + MySQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 10; + sess.default_hostgroup = 5; + sess.transaction_persistent_hostgroup = -1; + + MySQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = -1; // No rule match + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 0); // Legacy mode + ok(res.new_current_hostgroup == 5, "Legacy: falls back to default_hostgroup when no QPO destination"); + + sess.transaction_persistent_hostgroup = 10; + res = resolve_hostgroup_routing(sess, qpo, 0); + ok(res.new_current_hostgroup == 10, "Legacy: remains on transaction_persistent_hostgroup"); +} + +int main() { + plan(15); + + test_mirroring(); + test_show_warnings(); + test_last_insert_id(); + test_locking_success(); + test_locking_error(); + test_legacy_behavior(); + + return exit_status(); +} diff --git a/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp b/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp new file mode 100644 index 0000000000..a54df005f2 --- /dev/null +++ b/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp @@ -0,0 +1,92 @@ +#include +#include +#include "tap.h" +#include "PgSQL_HostGroup_Routing.h" + +// TAP noise tool stubs +extern "C" int get_noise_tools_count() { return 0; } +extern "C" void stop_noise_tools() {} +std::mutex noise_failure_mutex; +std::vector noise_failures; + +void test_basic_routing() { + PgSQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 10; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = -1; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = 20; + qpo.lock_hostgroup = false; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 20, "Basic: current_hostgroup should be destination_hostgroup from QPO"); + ok(res.error == false, "Basic: no error expected"); +} + +void test_locking_success() { + PgSQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 10; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = -1; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = 20; + qpo.lock_hostgroup = true; + + // Test initial locking + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 20, "Locking: current_hostgroup updated to destination"); + ok(res.lock_hostgroup == true, "Locking: lock_hostgroup flag set"); + ok(res.new_locked_on_hostgroup == 20, "Locking: new_locked_on_hostgroup set to 20"); + ok(res.error == false, "Locking: no error expected"); +} + +void test_locking_error() { + PgSQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 20; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = 20; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = 30; // Trying to reach a different hostgroup + qpo.lock_hostgroup = false; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); + ok(res.error == true, "Locked Error: error set when trying to reach different hostgroup"); + ok(res.error_msg.find("locked to hostgroup 20") != std::string::npos, "Locked Error: error message contains correct hostgroup"); +} + +void test_legacy_behavior() { + PgSQL_Routing_Session_State sess = {0}; + sess.current_hostgroup = 10; + sess.default_hostgroup = 5; + sess.locked_on_hostgroup = -1; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo = {0}; + qpo.destination_hostgroup = -1; // No rule match + qpo.lock_hostgroup = false; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 0); // Legacy mode + ok(res.new_current_hostgroup == 5, "Legacy: falls back to default_hostgroup when no QPO destination"); + + sess.transaction_persistent_hostgroup = 10; + res = resolve_pgsql_hostgroup_routing(sess, qpo, 0); + ok(res.new_current_hostgroup == 10, "Legacy: remains on transaction_persistent_hostgroup"); +} + +int main() { + plan(10); + + test_basic_routing(); + test_locking_success(); + test_locking_error(); + test_legacy_behavior(); + + return exit_status(); +} From f01c415ed7636228f01ffafdc8c777225acab0d0 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Tue, 24 Mar 2026 00:39:53 +0000 Subject: [PATCH 3/4] Address review feedback for hostgroup routing extraction - Restored existing unit tests in test/tap/tests/unit/Makefile that were accidentally removed. - Refactored new unit tests to use standard ProxySQL test harness (test_init_minimal()) and pattern build rule. - Fixed MySQL routing logic to correctly fallback to default_hostgroup when no QPO destination is provided. - Improved SHOW WARNINGS detection using strcasestr. - Added hostgroup lock counter increments in PgSQL extended-query paths to ensure consistency and prevent negative gauges. - Improved safety by replacing sprintf with snprintf for error messages. - Fixed SQLSTATE typo (Y0000 -> HY000). - Renamed struct members and variables to match project conventions. - Standardized include guards. Verified all 25 new test cases and restored existing unit tests pass. --- include/MySQL_HostGroup_Routing.h | 50 ++--- include/PgSQL_HostGroup_Routing.h | 34 +-- lib/MySQL_HostGroup_Routing.cpp | 124 ++++++----- lib/MySQL_Session.cpp | 51 +++-- lib/PgSQL_HostGroup_Routing.cpp | 84 ++++---- lib/PgSQL_Session.cpp | 20 +- test/tap/tests/unit/Makefile | 63 ++---- .../tests/unit/MySQL_HostGroup_Routing-t.cpp | 202 +++++++++--------- .../tests/unit/PgSQL_HostGroup_Routing-t.cpp | 144 ++++++------- 9 files changed, 373 insertions(+), 399 deletions(-) diff --git a/include/MySQL_HostGroup_Routing.h b/include/MySQL_HostGroup_Routing.h index 884bb5992e..35ade8df59 100644 --- a/include/MySQL_HostGroup_Routing.h +++ b/include/MySQL_HostGroup_Routing.h @@ -1,5 +1,5 @@ -#ifndef __MYSQL_HOSTGROUP_ROUTING_H -#define __MYSQL_HOSTGROUP_ROUTING_H +#ifndef MYSQL_HOSTGROUP_ROUTING_H +#define MYSQL_HOSTGROUP_ROUTING_H #include @@ -8,15 +8,15 @@ * @brief Represents the session state relevant for hostgroup routing decisions. */ struct MySQL_Routing_Session_State { - int current_hostgroup; - int default_hostgroup; - int locked_on_hostgroup; - int transaction_persistent_hostgroup; - int last_HG_affected_rows; - int warning_in_hg; - bool autocommit; - int autocommit_on_hostgroup; - bool mirror; + int current_hostgroup{-1}; + int default_hostgroup{-1}; + int locked_on_hostgroup{-1}; + int transaction_persistent_hostgroup{-1}; + int last_hg_affected_rows{-1}; + int warning_in_hg{-1}; + bool autocommit{true}; + int autocommit_on_hostgroup{-1}; + bool mirror{false}; }; /** @@ -24,11 +24,11 @@ struct MySQL_Routing_Session_State { * @brief Represents the Query Processor Output relevant for hostgroup routing decisions. */ struct MySQL_Routing_QPO_State { - int destination_hostgroup; - bool is_set_statement; // Derived from query parsing - bool is_show_warnings; // Derived from query parsing - bool is_last_insert_id; // Derived from query parsing - bool is_version_query; // Derived from query parsing + int destination_hostgroup{-1}; + bool lock_hostgroup{false}; // Derived from query parsing or QPO + bool is_show_warnings{false}; // Derived from query parsing + bool is_last_insert_id{false}; // Derived from query parsing + bool is_version_query{false}; // Derived from query parsing }; /** @@ -36,11 +36,11 @@ struct MySQL_Routing_QPO_State { * @brief Represents the output of the hostgroup routing decision. */ struct MySQL_Routing_Result { - int new_current_hostgroup; - int new_locked_on_hostgroup; - bool lock_hostgroup; - bool error; - std::string error_msg; + int new_current_hostgroup{-1}; + int new_locked_on_hostgroup{-1}; + bool lock_hostgroup{false}; + bool error{false}; + std::string error_msg; }; /** @@ -54,9 +54,9 @@ struct MySQL_Routing_Result { * @return MySQL_Routing_Result The routing decision. */ MySQL_Routing_Result resolve_hostgroup_routing( - const MySQL_Routing_Session_State& sess_state, - const MySQL_Routing_QPO_State& qpo_state, - int set_query_lock_on_hostgroup + const MySQL_Routing_Session_State& sess_state, + const MySQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup ); -#endif // __MYSQL_HOSTGROUP_ROUTING_H +#endif // MYSQL_HOSTGROUP_ROUTING_H diff --git a/include/PgSQL_HostGroup_Routing.h b/include/PgSQL_HostGroup_Routing.h index 9cc23777fb..353f0890d6 100644 --- a/include/PgSQL_HostGroup_Routing.h +++ b/include/PgSQL_HostGroup_Routing.h @@ -1,5 +1,5 @@ -#ifndef __PGSQL_HOSTGROUP_ROUTING_H -#define __PGSQL_HOSTGROUP_ROUTING_H +#ifndef PGSQL_HOSTGROUP_ROUTING_H +#define PGSQL_HOSTGROUP_ROUTING_H #include @@ -8,10 +8,10 @@ * @brief Represents the session state relevant for hostgroup routing decisions in PostgreSQL. */ struct PgSQL_Routing_Session_State { - int current_hostgroup; - int default_hostgroup; - int locked_on_hostgroup; - int transaction_persistent_hostgroup; + int current_hostgroup{-1}; + int default_hostgroup{-1}; + int locked_on_hostgroup{-1}; + int transaction_persistent_hostgroup{-1}; }; /** @@ -19,8 +19,8 @@ struct PgSQL_Routing_Session_State { * @brief Represents the Query Processor Output relevant for hostgroup routing decisions in PostgreSQL. */ struct PgSQL_Routing_QPO_State { - int destination_hostgroup; - bool lock_hostgroup; // Derived from query parsing + int destination_hostgroup{-1}; + bool lock_hostgroup{false}; // Derived from query parsing }; /** @@ -28,11 +28,11 @@ struct PgSQL_Routing_QPO_State { * @brief Represents the output of the hostgroup routing decision for PostgreSQL. */ struct PgSQL_Routing_Result { - int new_current_hostgroup; - int new_locked_on_hostgroup; - bool lock_hostgroup; - bool error; - std::string error_msg; + int new_current_hostgroup{-1}; + int new_locked_on_hostgroup{-1}; + bool lock_hostgroup{false}; + bool error{false}; + std::string error_msg; }; /** @@ -46,9 +46,9 @@ struct PgSQL_Routing_Result { * @return PgSQL_Routing_Result The routing decision. */ PgSQL_Routing_Result resolve_pgsql_hostgroup_routing( - const PgSQL_Routing_Session_State& sess_state, - const PgSQL_Routing_QPO_State& qpo_state, - int set_query_lock_on_hostgroup + const PgSQL_Routing_Session_State& sess_state, + const PgSQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup ); -#endif // __PGSQL_HOSTGROUP_ROUTING_H +#endif // PGSQL_HOSTGROUP_ROUTING_H diff --git a/lib/MySQL_HostGroup_Routing.cpp b/lib/MySQL_HostGroup_Routing.cpp index 3e1e98ffca..48406498c9 100644 --- a/lib/MySQL_HostGroup_Routing.cpp +++ b/lib/MySQL_HostGroup_Routing.cpp @@ -1,76 +1,72 @@ #include "MySQL_HostGroup_Routing.h" MySQL_Routing_Result resolve_hostgroup_routing( - const MySQL_Routing_Session_State& sess_state, - const MySQL_Routing_QPO_State& qpo_state, - int set_query_lock_on_hostgroup + const MySQL_Routing_Session_State& sess_state, + const MySQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup ) { - MySQL_Routing_Result result; - result.new_current_hostgroup = sess_state.current_hostgroup; - result.new_locked_on_hostgroup = sess_state.locked_on_hostgroup; - result.lock_hostgroup = false; - result.error = false; - result.error_msg = ""; + MySQL_Routing_Result result; + result.new_current_hostgroup = sess_state.current_hostgroup; + result.new_locked_on_hostgroup = sess_state.locked_on_hostgroup; + result.lock_hostgroup = false; + result.error = false; + result.error_msg = ""; - // 1. Mirroring - if (sess_state.mirror) { - result.new_current_hostgroup = qpo_state.destination_hostgroup; - return result; - } + // 1. Mirroring (highest priority) + if (sess_state.mirror) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + return result; + } - // 2. SHOW WARNINGS / SHOW COUNT(*) WARNINGS - if (qpo_state.is_show_warnings) { - if (sess_state.warning_in_hg > -1) { - result.new_current_hostgroup = sess_state.warning_in_hg; - } - return result; - } + // 2. SHOW WARNINGS / SHOW COUNT(*) WARNINGS + if (qpo_state.is_show_warnings) { + if (sess_state.warning_in_hg > -1) { + result.new_current_hostgroup = sess_state.warning_in_hg; + } + return result; + } - // 3. LAST_INSERT_ID / @@IDENTITY - if (qpo_state.is_last_insert_id) { - if (sess_state.last_HG_affected_rows >= 0) { - result.new_current_hostgroup = sess_state.last_HG_affected_rows; - return result; - } - } + // 3. LAST_INSERT_ID / @@IDENTITY + if (qpo_state.is_last_insert_id) { + if (sess_state.last_hg_affected_rows >= 0) { + result.new_current_hostgroup = sess_state.last_hg_affected_rows; + return result; + } + } - // 4. Default routing from QPO - if (qpo_state.destination_hostgroup >= 0) { - if (sess_state.transaction_persistent_hostgroup == -1) { - result.new_current_hostgroup = qpo_state.destination_hostgroup; - } - } + // 4. Default routing from QPO + if (qpo_state.destination_hostgroup >= 0) { + if (sess_state.transaction_persistent_hostgroup == -1) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + } + } else { + // qpo_state.destination_hostgroup < 0 means no override from QPO + if (sess_state.transaction_persistent_hostgroup == -1) { + result.new_current_hostgroup = sess_state.default_hostgroup; + } + } - // 5. Hostgroup Locking Decisions (mysql-set_query_lock_on_hostgroup) - if (set_query_lock_on_hostgroup == 1) { - // Algorithm introduced in ProxySQL 2.0.6 - if (result.new_locked_on_hostgroup < 0) { - if (qpo_state.is_set_statement) { - // If it's a SET statement that caused locking (determined by parser) - // In a pure function, we assume qpo_state.is_set_statement implies it should lock - result.lock_hostgroup = true; - result.new_locked_on_hostgroup = result.new_current_hostgroup; - } - } + // 5. Hostgroup Locking Decisions (mysql-set_query_lock_on_hostgroup) + if (set_query_lock_on_hostgroup == 1) { + // Algorithm introduced in ProxySQL 2.0.6 + if (result.new_locked_on_hostgroup < 0) { + if (qpo_state.lock_hostgroup) { + result.lock_hostgroup = true; + result.new_locked_on_hostgroup = result.new_current_hostgroup; + } + } - if (result.new_locked_on_hostgroup >= 0) { - if (result.new_current_hostgroup != result.new_locked_on_hostgroup) { - result.error = true; - result.error_msg = "ProxySQL Error: connection is locked to hostgroup " + - std::to_string(result.new_locked_on_hostgroup) + - " but trying to reach hostgroup " + - std::to_string(result.new_current_hostgroup); - return result; - } - } - } else { - // Legacy behavior before 2.0.6 - if (sess_state.transaction_persistent_hostgroup == -1) { - if (qpo_state.destination_hostgroup < 0) { - result.new_current_hostgroup = sess_state.default_hostgroup; - } - } - } + if (result.new_locked_on_hostgroup >= 0) { + if (result.new_current_hostgroup != result.new_locked_on_hostgroup) { + result.error = true; + result.error_msg = "ProxySQL Error: connection is locked to hostgroup " + + std::to_string(result.new_locked_on_hostgroup) + + " but trying to reach hostgroup " + + std::to_string(result.new_current_hostgroup); + return result; + } + } + } - return result; + return result; } diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index 2c9edf2399..edf63b764c 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -3336,7 +3336,7 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C sess_state.default_hostgroup = default_hostgroup; sess_state.locked_on_hostgroup = locked_on_hostgroup; sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; - sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.last_hg_affected_rows = last_HG_affected_rows; sess_state.warning_in_hg = warning_in_hg; sess_state.autocommit = autocommit; sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; @@ -3344,11 +3344,10 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C MySQL_Routing_QPO_State qpo_state = {0}; qpo_state.destination_hostgroup = qpo->destination_hostgroup; - qpo_state.is_set_statement = lock_hostgroup; + qpo_state.lock_hostgroup = lock_hostgroup; if (CurrentQuery.QueryParserArgs.digest_text) { const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; - const size_t dig_len = strlen(dig_text); - if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + if (strcasestr(dig_text, "SHOW WARNINGS") || strcasestr(dig_text, "SHOW COUNT(*) WARNINGS")) { qpo_state.is_show_warnings = true; } if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { @@ -3367,9 +3366,10 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C end = (char *)"..."; } string nqn = string((char *)CurrentQuery.QueryPointer,l); - char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; - char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); - sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + const char *err_msg = "Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + size_t buf_size = strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64; + char *buf = (char *)malloc(buf_size); + snprintf(buf, buf_size, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; RequestEnd(NULL, 9005, buf); @@ -3535,7 +3535,7 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C sess_state.default_hostgroup = default_hostgroup; sess_state.locked_on_hostgroup = locked_on_hostgroup; sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; - sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.last_hg_affected_rows = last_HG_affected_rows; sess_state.warning_in_hg = warning_in_hg; sess_state.autocommit = autocommit; sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; @@ -3543,11 +3543,10 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C MySQL_Routing_QPO_State qpo_state = {0}; qpo_state.destination_hostgroup = qpo->destination_hostgroup; - qpo_state.is_set_statement = lock_hostgroup; + qpo_state.lock_hostgroup = lock_hostgroup; if (CurrentQuery.QueryParserArgs.digest_text) { const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; - const size_t dig_len = strlen(dig_text); - if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + if (strcasestr(dig_text, "SHOW WARNINGS") || strcasestr(dig_text, "SHOW COUNT(*) WARNINGS")) { qpo_state.is_show_warnings = true; } if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { @@ -3571,9 +3570,10 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C end = (char *)"..."; } string nqn = string((char *)CurrentQuery.stmt_info->query,l); - char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; - char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); - sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + const char *err_msg = "Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + size_t buf_size = strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64; + char *buf = (char *)malloc(buf_size); + snprintf(buf, buf_size, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; RequestEnd(NULL, 9005, buf); @@ -5408,7 +5408,7 @@ int MySQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { sess_state.default_hostgroup = default_hostgroup; sess_state.locked_on_hostgroup = locked_on_hostgroup; sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; - sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.last_hg_affected_rows = last_HG_affected_rows; sess_state.warning_in_hg = warning_in_hg; sess_state.autocommit = autocommit; sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; @@ -5416,11 +5416,10 @@ int MySQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { MySQL_Routing_QPO_State qpo_state = {0}; qpo_state.destination_hostgroup = qpo->destination_hostgroup; - qpo_state.is_set_statement = lock_hostgroup; + qpo_state.lock_hostgroup = lock_hostgroup; if (CurrentQuery.QueryParserArgs.digest_text) { const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; - const size_t dig_len = strlen(dig_text); - if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + if (strcasestr(dig_text, "SHOW WARNINGS") || strcasestr(dig_text, "SHOW COUNT(*) WARNINGS")) { qpo_state.is_show_warnings = true; } if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { @@ -5439,9 +5438,10 @@ int MySQL_Session::get_pkts_from_client(bool& wrong_pass, PtrSize_t& pkt) { end = (char *)"..."; } string nqn = string((char *)CurrentQuery.QueryPointer,l); - char *err_msg = (char *)"Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; - char *buf = (char *)malloc(strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64); - sprintf(buf, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); + const char *err_msg = "Session trying to reach HG %d while locked on HG %d . Rejecting query: %s%s"; + size_t buf_size = strlen(err_msg)+strlen(nqn.c_str())+strlen(end)+64; + char *buf = (char *)malloc(buf_size); + snprintf(buf, buf_size, err_msg, res.new_current_hostgroup, locked_on_hostgroup, nqn.c_str(), end); client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9005,(char *)"HY000",buf, true); thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; RequestEnd(NULL, 9005, buf); @@ -8351,7 +8351,7 @@ bool MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C sess_state.default_hostgroup = default_hostgroup; sess_state.locked_on_hostgroup = locked_on_hostgroup; sess_state.transaction_persistent_hostgroup = transaction_persistent_hostgroup; - sess_state.last_HG_affected_rows = last_HG_affected_rows; + sess_state.last_hg_affected_rows = last_HG_affected_rows; sess_state.warning_in_hg = warning_in_hg; sess_state.autocommit = autocommit; sess_state.autocommit_on_hostgroup = autocommit_on_hostgroup; @@ -8359,11 +8359,10 @@ bool MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C MySQL_Routing_QPO_State qpo_state = {0}; qpo_state.destination_hostgroup = qpo->destination_hostgroup; - qpo_state.is_set_statement = *lock_hostgroup; + qpo_state.lock_hostgroup = *lock_hostgroup; if (CurrentQuery.QueryParserArgs.digest_text) { const char* dig_text = CurrentQuery.QueryParserArgs.digest_text; - const size_t dig_len = strlen(dig_text); - if ((dig_len == 13) && (strncasecmp(dig_text, "SHOW WARNINGS", 13) == 0)) { + if (strcasestr(dig_text, "SHOW WARNINGS") || strcasestr(dig_text, "SHOW COUNT(*) WARNINGS")) { qpo_state.is_show_warnings = true; } if (strcasestr(dig_text,"LAST_INSERT_ID") || strcasestr(dig_text,"@@IDENTITY")) { @@ -8375,7 +8374,7 @@ bool MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C if (res.error) { client_myds->DSS=STATE_QUERY_SENT_NET; - client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9006,(char *)"Y0000", (char*)res.error_msg.c_str()); + client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,client_myds->pkt_sid+1,9006,(char *)"HY000", (char*)res.error_msg.c_str()); thread->status_variables.stvar[st_var_hostgroup_locked_queries]++; RequestEnd(NULL, 9006, (char*)res.error_msg.c_str()); l_free(pkt->size,pkt->ptr); diff --git a/lib/PgSQL_HostGroup_Routing.cpp b/lib/PgSQL_HostGroup_Routing.cpp index bf14017b27..e671f4364d 100644 --- a/lib/PgSQL_HostGroup_Routing.cpp +++ b/lib/PgSQL_HostGroup_Routing.cpp @@ -1,52 +1,50 @@ #include "PgSQL_HostGroup_Routing.h" PgSQL_Routing_Result resolve_pgsql_hostgroup_routing( - const PgSQL_Routing_Session_State& sess_state, - const PgSQL_Routing_QPO_State& qpo_state, - int set_query_lock_on_hostgroup + const PgSQL_Routing_Session_State& sess_state, + const PgSQL_Routing_QPO_State& qpo_state, + int set_query_lock_on_hostgroup ) { - PgSQL_Routing_Result result; - result.new_current_hostgroup = sess_state.current_hostgroup; - result.new_locked_on_hostgroup = sess_state.locked_on_hostgroup; - result.lock_hostgroup = false; - result.error = false; - result.error_msg = ""; + PgSQL_Routing_Result result; + result.new_current_hostgroup = sess_state.current_hostgroup; + result.new_locked_on_hostgroup = sess_state.locked_on_hostgroup; + result.lock_hostgroup = false; + result.error = false; + result.error_msg = ""; - // Default routing from QPO - if (qpo_state.destination_hostgroup >= 0) { - if (sess_state.transaction_persistent_hostgroup == -1) { - result.new_current_hostgroup = qpo_state.destination_hostgroup; - } - } + // 1. Default routing from QPO + if (qpo_state.destination_hostgroup >= 0) { + if (sess_state.transaction_persistent_hostgroup == -1) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + } + } else { + // qpo_state.destination_hostgroup < 0 means no override from QPO + if (sess_state.transaction_persistent_hostgroup == -1) { + result.new_current_hostgroup = sess_state.default_hostgroup; + } + } - // Hostgroup Locking Decisions (pgsql-set_query_lock_on_hostgroup) - if (set_query_lock_on_hostgroup == 1) { - // Algorithm introduced in ProxySQL 2.0.6 - if (result.new_locked_on_hostgroup < 0) { - if (qpo_state.lock_hostgroup) { - result.lock_hostgroup = true; - result.new_locked_on_hostgroup = result.new_current_hostgroup; - } - } + // 2. Hostgroup Locking Decisions (pgsql-set_query_lock_on_hostgroup) + if (set_query_lock_on_hostgroup == 1) { + // Algorithm introduced in ProxySQL 2.0.6 + if (result.new_locked_on_hostgroup < 0) { + if (qpo_state.lock_hostgroup) { + result.lock_hostgroup = true; + result.new_locked_on_hostgroup = result.new_current_hostgroup; + } + } - if (result.new_locked_on_hostgroup >= 0) { - if (result.new_current_hostgroup != result.new_locked_on_hostgroup) { - result.error = true; - result.error_msg = "ProxySQL Error: connection is locked to hostgroup " + - std::to_string(result.new_locked_on_hostgroup) + - " but trying to reach hostgroup " + - std::to_string(result.new_current_hostgroup); - return result; - } - } - } else { - // Legacy behavior before 2.0.6 - if (sess_state.transaction_persistent_hostgroup == -1) { - if (qpo_state.destination_hostgroup < 0) { - result.new_current_hostgroup = sess_state.default_hostgroup; - } - } - } + if (result.new_locked_on_hostgroup >= 0) { + if (result.new_current_hostgroup != result.new_locked_on_hostgroup) { + result.error = true; + result.error_msg = "ProxySQL Error: connection is locked to hostgroup " + + std::to_string(result.new_locked_on_hostgroup) + + " but trying to reach hostgroup " + + std::to_string(result.new_current_hostgroup); + return result; + } + } + } - return result; + return result; } diff --git a/lib/PgSQL_Session.cpp b/lib/PgSQL_Session.cpp index f80380bdd7..bc6584445c 100644 --- a/lib/PgSQL_Session.cpp +++ b/lib/PgSQL_Session.cpp @@ -4749,6 +4749,10 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___PGSQL_Q current_hostgroup = res.new_current_hostgroup; locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; + } } return false; @@ -6145,8 +6149,8 @@ int PgSQL_Session::handle_post_sync_parse_message(PgSQL_Parse_Message* parse_msg current_hostgroup = res.new_current_hostgroup; locked_on_hostgroup = res.new_locked_on_hostgroup; if (res.lock_hostgroup) { - // PgSQL session doesn't seem to update status variables here like MySQL does, - // but we keep the logic consistent. + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; } } @@ -6401,6 +6405,10 @@ int PgSQL_Session::handle_post_sync_describe_message(PgSQL_Describe_Message* des current_hostgroup = res.new_current_hostgroup; locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; + } } if (extended_query_frame.empty() == true) { @@ -6554,6 +6562,10 @@ int PgSQL_Session::handle_post_sync_bind_message(PgSQL_Bind_Message* bind_msg) { current_hostgroup = res.new_current_hostgroup; locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; + } } bind_waiting_for_execute.reset(bind_msg->release()); // release the ownership of the bind message @@ -6702,6 +6714,10 @@ int PgSQL_Session::handle_post_sync_execute_message(PgSQL_Execute_Message* execu current_hostgroup = res.new_current_hostgroup; locked_on_hostgroup = res.new_locked_on_hostgroup; + if (res.lock_hostgroup) { + thread->status_variables.stvar[st_var_hostgroup_locked]++; + thread->status_variables.stvar[st_var_hostgroup_locked_set_cmds]++; + } } if (extended_query_frame.empty() == true) { diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index 80fe97f141..45801c4245 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -231,7 +231,20 @@ $(ODIR)/test_init.o: $(TEST_HELPERS_DIR)/test_init.cpp | $(ODIR) # Unit test targets # =========================================================================== -UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t protocol_unit-t auth_unit-t connection_pool_unit-t rule_matching_unit-t hostgroups_unit-t monitor_health_unit-t MySQL_HostGroup_Routing-t PgSQL_HostGroup_Routing-t +UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t \ + protocol_unit-t auth_unit-t connection_pool_unit-t \ + rule_matching_unit-t hostgroups_unit-t monitor_health_unit-t \ + pgsql_command_complete_unit-t \ + ffto_protocol_unit-t \ + server_selection_unit-t \ + hostgroup_routing_unit-t \ + transaction_state_unit-t \ + pgsql_error_classifier_unit-t \ + pgsql_monitor_unit-t \ + mysql_error_classifier_unit-t \ + backend_sync_unit-t \ + MySQL_HostGroup_Routing-t \ + PgSQL_HostGroup_Routing-t .PHONY: all all: $(UNIT_TESTS) @@ -245,57 +258,19 @@ ifneq ($(UNAME_S),Darwin) ALLOW_MULTI_DEF := -Wl,--allow-multiple-definition endif -smoke_test-t: smoke_test-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) +# Pattern rule: all unit tests use the same compile + link flags. +# Each test binary is built from its .cpp source, linked against +# the test harness objects and libproxysql.a with all dependencies. +MySQL_HostGroup_Routing-t: MySQL_HostGroup_Routing-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ $(ALLOW_MULTI_DEF) -o $@ -query_cache_unit-t: query_cache_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) +PgSQL_HostGroup_Routing-t: PgSQL_HostGroup_Routing-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ $(ALLOW_MULTI_DEF) -o $@ -query_processor_unit-t: query_processor_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -protocol_unit-t: protocol_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -auth_unit-t: auth_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -connection_pool_unit-t: connection_pool_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -rule_matching_unit-t: rule_matching_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -hostgroups_unit-t: hostgroups_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -monitor_health_unit-t: monitor_health_unit-t.cpp $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR) - $(CXX) $< $(TEST_HELPERS_OBJ) $(IDIRS) $(LDIRS) $(OPT) \ - $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) $(MYLIBS) \ - $(ALLOW_MULTI_DEF) -o $@ - -MySQL_HostGroup_Routing-t: MySQL_HostGroup_Routing-t.cpp $(PROXYSQL_PATH)/lib/MySQL_HostGroup_Routing.cpp $(ODIR)/tap.o - $(CXX) $^ $(IDIRS) $(OPT) -o $@ - -PgSQL_HostGroup_Routing-t: PgSQL_HostGroup_Routing-t.cpp $(PROXYSQL_PATH)/lib/PgSQL_HostGroup_Routing.cpp $(ODIR)/tap.o - $(CXX) $^ $(IDIRS) $(OPT) -o $@ - # =========================================================================== # Clean diff --git a/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp b/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp index 9b945e739d..294ef8949b 100644 --- a/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp +++ b/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp @@ -1,126 +1,122 @@ -#include -#include #include "tap.h" +#include "test_globals.h" +#include "test_init.h" #include "MySQL_HostGroup_Routing.h" -// TAP noise tool stubs -extern "C" int get_noise_tools_count() { return 0; } -extern "C" void stop_noise_tools() {} -std::mutex noise_failure_mutex; -std::vector noise_failures; - void test_mirroring() { - MySQL_Routing_Session_State sess = {0}; - sess.mirror = true; - sess.current_hostgroup = 10; - - MySQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = 20; - - MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); - - ok(res.new_current_hostgroup == 20, "Mirroring: current_hostgroup should be destination_hostgroup from QPO"); - ok(res.error == false, "Mirroring: no error expected"); + MySQL_Routing_Session_State sess; + sess.mirror = true; + sess.current_hostgroup = 10; + + MySQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = 20; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 20, "Mirroring: current_hostgroup should be destination_hostgroup from QPO"); + ok(res.error == false, "Mirroring: no error expected"); } void test_show_warnings() { - MySQL_Routing_Session_State sess = {0}; - sess.warning_in_hg = 15; - sess.current_hostgroup = 10; - - MySQL_Routing_QPO_State qpo = {0}; - qpo.is_show_warnings = true; - - MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); - - ok(res.new_current_hostgroup == 15, "SHOW WARNINGS: current_hostgroup should be warning_in_hg"); - - sess.warning_in_hg = -1; - res = resolve_hostgroup_routing(sess, qpo, 1); - ok(res.new_current_hostgroup == 10, "SHOW WARNINGS: current_hostgroup should remain unchanged if warning_in_hg is -1"); + MySQL_Routing_Session_State sess; + sess.warning_in_hg = 15; + sess.current_hostgroup = 10; + + MySQL_Routing_QPO_State qpo; + qpo.is_show_warnings = true; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 15, "SHOW WARNINGS: current_hostgroup should be warning_in_hg"); + + sess.warning_in_hg = -1; + res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 10, "SHOW WARNINGS: current_hostgroup should remain unchanged if warning_in_hg is -1"); } void test_last_insert_id() { - MySQL_Routing_Session_State sess = {0}; - sess.last_HG_affected_rows = 25; - sess.current_hostgroup = 10; - - MySQL_Routing_QPO_State qpo = {0}; - qpo.is_last_insert_id = true; - - MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); - - ok(res.new_current_hostgroup == 25, "LAST_INSERT_ID: current_hostgroup should be last_HG_affected_rows"); + MySQL_Routing_Session_State sess; + sess.last_hg_affected_rows = 25; + sess.current_hostgroup = 10; + + MySQL_Routing_QPO_State qpo; + qpo.is_last_insert_id = true; + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 25, "LAST_INSERT_ID: current_hostgroup should be last_hg_affected_rows"); } void test_locking_success() { - MySQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 10; - sess.default_hostgroup = 10; - sess.locked_on_hostgroup = -1; - sess.transaction_persistent_hostgroup = -1; - - MySQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = 20; - qpo.is_set_statement = true; - - // Test initial locking - MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); - ok(res.new_current_hostgroup == 20, "Locking: current_hostgroup updated to destination (expected 20, got %d)", res.new_current_hostgroup); - ok(res.lock_hostgroup == true, "Locking: lock_hostgroup flag set"); - ok(res.new_locked_on_hostgroup == 20, "Locking: new_locked_on_hostgroup set to 20 (got %d)", res.new_locked_on_hostgroup); - ok(res.error == false, "Locking: no error expected"); - - // Test subsequent query on same hostgroup - sess.current_hostgroup = 20; - sess.locked_on_hostgroup = 20; - qpo.is_set_statement = false; - qpo.destination_hostgroup = 20; - res = resolve_hostgroup_routing(sess, qpo, 1); - ok(res.new_current_hostgroup == 20, "Locked: current_hostgroup remains 20"); - ok(res.error == false, "Locked: no error when hostgroup matches"); + MySQL_Routing_Session_State sess; + sess.current_hostgroup = 10; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = -1; + sess.transaction_persistent_hostgroup = -1; + + MySQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = 20; + qpo.lock_hostgroup = true; + + // Test initial locking + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 20, "Locking: current_hostgroup updated to destination (expected 20, got %d)", res.new_current_hostgroup); + ok(res.lock_hostgroup == true, "Locking: lock_hostgroup flag set"); + ok(res.new_locked_on_hostgroup == 20, "Locking: new_locked_on_hostgroup set to 20 (got %d)", res.new_locked_on_hostgroup); + ok(res.error == false, "Locking: no error expected"); + + // Test subsequent query on same hostgroup + sess.current_hostgroup = 20; + sess.locked_on_hostgroup = 20; + qpo.lock_hostgroup = false; + qpo.destination_hostgroup = 20; + res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 20, "Locked: current_hostgroup remains 20"); + ok(res.error == false, "Locked: no error when hostgroup matches"); } void test_locking_error() { - MySQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 20; - sess.locked_on_hostgroup = 20; - sess.transaction_persistent_hostgroup = -1; - - MySQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = 30; // Trying to reach a different hostgroup - - MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); - ok(res.error == true, "Locked Error: error set when trying to reach different hostgroup"); - ok(res.error_msg.find("locked to hostgroup 20") != std::string::npos, "Locked Error: error message contains correct hostgroup"); + MySQL_Routing_Session_State sess; + sess.current_hostgroup = 20; + sess.locked_on_hostgroup = 20; + sess.transaction_persistent_hostgroup = -1; + + MySQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = 30; // Trying to reach a different hostgroup + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 1); + ok(res.error == true, "Locked Error: error set when trying to reach different hostgroup"); + ok(res.error_msg.find("locked to hostgroup 20") != std::string::npos, "Locked Error: error message contains correct hostgroup"); } void test_legacy_behavior() { - MySQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 10; - sess.default_hostgroup = 5; - sess.transaction_persistent_hostgroup = -1; - - MySQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = -1; // No rule match - - MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 0); // Legacy mode - ok(res.new_current_hostgroup == 5, "Legacy: falls back to default_hostgroup when no QPO destination"); - - sess.transaction_persistent_hostgroup = 10; - res = resolve_hostgroup_routing(sess, qpo, 0); - ok(res.new_current_hostgroup == 10, "Legacy: remains on transaction_persistent_hostgroup"); + MySQL_Routing_Session_State sess; + sess.current_hostgroup = 10; + sess.default_hostgroup = 5; + sess.transaction_persistent_hostgroup = -1; + + MySQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = -1; // No rule match + + MySQL_Routing_Result res = resolve_hostgroup_routing(sess, qpo, 0); // Legacy mode + ok(res.new_current_hostgroup == 5, "Legacy: falls back to default_hostgroup when no QPO destination"); + + sess.transaction_persistent_hostgroup = 10; + res = resolve_hostgroup_routing(sess, qpo, 0); + ok(res.new_current_hostgroup == 10, "Legacy: remains on transaction_persistent_hostgroup"); } int main() { - plan(15); - - test_mirroring(); - test_show_warnings(); - test_last_insert_id(); - test_locking_success(); - test_locking_error(); - test_legacy_behavior(); - - return exit_status(); + plan(15); + test_init_minimal(); + + test_mirroring(); + test_show_warnings(); + test_last_insert_id(); + test_locking_success(); + test_locking_error(); + test_legacy_behavior(); + + test_cleanup_minimal(); + return exit_status(); } diff --git a/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp b/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp index a54df005f2..d50d499fdf 100644 --- a/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp +++ b/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp @@ -1,92 +1,86 @@ -#include -#include #include "tap.h" +#include "test_globals.h" +#include "test_init.h" #include "PgSQL_HostGroup_Routing.h" -// TAP noise tool stubs -extern "C" int get_noise_tools_count() { return 0; } -extern "C" void stop_noise_tools() {} -std::mutex noise_failure_mutex; -std::vector noise_failures; - void test_basic_routing() { - PgSQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 10; - sess.default_hostgroup = 10; - sess.locked_on_hostgroup = -1; - sess.transaction_persistent_hostgroup = -1; - - PgSQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = 20; - qpo.lock_hostgroup = false; - - PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); - - ok(res.new_current_hostgroup == 20, "Basic: current_hostgroup should be destination_hostgroup from QPO"); - ok(res.error == false, "Basic: no error expected"); + PgSQL_Routing_Session_State sess; + sess.current_hostgroup = 10; + sess.default_hostgroup = 10; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = 20; + qpo.lock_hostgroup = false; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); + + ok(res.new_current_hostgroup == 20, "Basic: current_hostgroup should be destination_hostgroup from QPO"); + ok(res.error == false, "Basic: no error expected"); } void test_locking_success() { - PgSQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 10; - sess.default_hostgroup = 10; - sess.locked_on_hostgroup = -1; - sess.transaction_persistent_hostgroup = -1; - - PgSQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = 20; - qpo.lock_hostgroup = true; - - // Test initial locking - PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); - ok(res.new_current_hostgroup == 20, "Locking: current_hostgroup updated to destination"); - ok(res.lock_hostgroup == true, "Locking: lock_hostgroup flag set"); - ok(res.new_locked_on_hostgroup == 20, "Locking: new_locked_on_hostgroup set to 20"); - ok(res.error == false, "Locking: no error expected"); + PgSQL_Routing_Session_State sess; + sess.current_hostgroup = 10; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = -1; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = 20; + qpo.lock_hostgroup = true; + + // Test initial locking + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); + ok(res.new_current_hostgroup == 20, "Locking: current_hostgroup updated to destination"); + ok(res.lock_hostgroup == true, "Locking: lock_hostgroup flag set"); + ok(res.new_locked_on_hostgroup == 20, "Locking: new_locked_on_hostgroup set to 20"); + ok(res.error == false, "Locking: no error expected"); } void test_locking_error() { - PgSQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 20; - sess.default_hostgroup = 10; - sess.locked_on_hostgroup = 20; - sess.transaction_persistent_hostgroup = -1; - - PgSQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = 30; // Trying to reach a different hostgroup - qpo.lock_hostgroup = false; - - PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); - ok(res.error == true, "Locked Error: error set when trying to reach different hostgroup"); - ok(res.error_msg.find("locked to hostgroup 20") != std::string::npos, "Locked Error: error message contains correct hostgroup"); + PgSQL_Routing_Session_State sess; + sess.current_hostgroup = 20; + sess.default_hostgroup = 10; + sess.locked_on_hostgroup = 20; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = 30; // Trying to reach a different hostgroup + qpo.lock_hostgroup = false; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 1); + ok(res.error == true, "Locked Error: error set when trying to reach different hostgroup"); + ok(res.error_msg.find("locked to hostgroup 20") != std::string::npos, "Locked Error: error message contains correct hostgroup"); } void test_legacy_behavior() { - PgSQL_Routing_Session_State sess = {0}; - sess.current_hostgroup = 10; - sess.default_hostgroup = 5; - sess.locked_on_hostgroup = -1; - sess.transaction_persistent_hostgroup = -1; - - PgSQL_Routing_QPO_State qpo = {0}; - qpo.destination_hostgroup = -1; // No rule match - qpo.lock_hostgroup = false; - - PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 0); // Legacy mode - ok(res.new_current_hostgroup == 5, "Legacy: falls back to default_hostgroup when no QPO destination"); - - sess.transaction_persistent_hostgroup = 10; - res = resolve_pgsql_hostgroup_routing(sess, qpo, 0); - ok(res.new_current_hostgroup == 10, "Legacy: remains on transaction_persistent_hostgroup"); + PgSQL_Routing_Session_State sess; + sess.current_hostgroup = 10; + sess.default_hostgroup = 5; + sess.transaction_persistent_hostgroup = -1; + + PgSQL_Routing_QPO_State qpo; + qpo.destination_hostgroup = -1; // No rule match + qpo.lock_hostgroup = false; + + PgSQL_Routing_Result res = resolve_pgsql_hostgroup_routing(sess, qpo, 0); // Legacy mode + ok(res.new_current_hostgroup == 5, "Legacy: falls back to default_hostgroup when no QPO destination"); + + sess.transaction_persistent_hostgroup = 10; + res = resolve_pgsql_hostgroup_routing(sess, qpo, 0); + ok(res.new_current_hostgroup == 10, "Legacy: remains on transaction_persistent_hostgroup"); } int main() { - plan(10); - - test_basic_routing(); - test_locking_success(); - test_locking_error(); - test_legacy_behavior(); - - return exit_status(); + plan(10); + test_init_minimal(); + + test_basic_routing(); + test_locking_success(); + test_locking_error(); + test_legacy_behavior(); + + test_cleanup_minimal(); + return exit_status(); } From f3be8cbf3f9c77d3b9185a87bb32e4661f296f23 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Tue, 24 Mar 2026 00:52:12 +0000 Subject: [PATCH 4/4] Improve routing logic for transaction persistence and fix test assertions - Updated resolve_hostgroup_routing and resolve_pgsql_hostgroup_routing to explicitly honor transaction_persistent_hostgroup if set. - Improved test_legacy_behavior in unit tests to use non-overlapping hostgroup IDs, ensuring assertions are not vacuous and truly verify persistence logic. - Verified all unit tests pass after logic updates. --- lib/MySQL_HostGroup_Routing.cpp | 14 +++++++------- lib/PgSQL_HostGroup_Routing.cpp | 14 +++++++------- test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp | 2 +- test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/MySQL_HostGroup_Routing.cpp b/lib/MySQL_HostGroup_Routing.cpp index 48406498c9..be464f3f01 100644 --- a/lib/MySQL_HostGroup_Routing.cpp +++ b/lib/MySQL_HostGroup_Routing.cpp @@ -34,14 +34,14 @@ MySQL_Routing_Result resolve_hostgroup_routing( } } - // 4. Default routing from QPO - if (qpo_state.destination_hostgroup >= 0) { - if (sess_state.transaction_persistent_hostgroup == -1) { - result.new_current_hostgroup = qpo_state.destination_hostgroup; - } + // 4. Default routing from QPO or Transaction Persistence + if (sess_state.transaction_persistent_hostgroup != -1) { + result.new_current_hostgroup = sess_state.transaction_persistent_hostgroup; } else { - // qpo_state.destination_hostgroup < 0 means no override from QPO - if (sess_state.transaction_persistent_hostgroup == -1) { + if (qpo_state.destination_hostgroup >= 0) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + } else { + // qpo_state.destination_hostgroup < 0 means no override from QPO result.new_current_hostgroup = sess_state.default_hostgroup; } } diff --git a/lib/PgSQL_HostGroup_Routing.cpp b/lib/PgSQL_HostGroup_Routing.cpp index e671f4364d..b5f61399bc 100644 --- a/lib/PgSQL_HostGroup_Routing.cpp +++ b/lib/PgSQL_HostGroup_Routing.cpp @@ -12,14 +12,14 @@ PgSQL_Routing_Result resolve_pgsql_hostgroup_routing( result.error = false; result.error_msg = ""; - // 1. Default routing from QPO - if (qpo_state.destination_hostgroup >= 0) { - if (sess_state.transaction_persistent_hostgroup == -1) { - result.new_current_hostgroup = qpo_state.destination_hostgroup; - } + // 1. Default routing from QPO or Transaction Persistence + if (sess_state.transaction_persistent_hostgroup != -1) { + result.new_current_hostgroup = sess_state.transaction_persistent_hostgroup; } else { - // qpo_state.destination_hostgroup < 0 means no override from QPO - if (sess_state.transaction_persistent_hostgroup == -1) { + if (qpo_state.destination_hostgroup >= 0) { + result.new_current_hostgroup = qpo_state.destination_hostgroup; + } else { + // qpo_state.destination_hostgroup < 0 means no override from QPO result.new_current_hostgroup = sess_state.default_hostgroup; } } diff --git a/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp b/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp index 294ef8949b..59fa50be51 100644 --- a/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp +++ b/test/tap/tests/unit/MySQL_HostGroup_Routing-t.cpp @@ -91,7 +91,7 @@ void test_locking_error() { void test_legacy_behavior() { MySQL_Routing_Session_State sess; - sess.current_hostgroup = 10; + sess.current_hostgroup = 99; sess.default_hostgroup = 5; sess.transaction_persistent_hostgroup = -1; diff --git a/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp b/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp index d50d499fdf..e329e9aeda 100644 --- a/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp +++ b/test/tap/tests/unit/PgSQL_HostGroup_Routing-t.cpp @@ -56,7 +56,7 @@ void test_locking_error() { void test_legacy_behavior() { PgSQL_Routing_Session_State sess; - sess.current_hostgroup = 10; + sess.current_hostgroup = 99; sess.default_hostgroup = 5; sess.transaction_persistent_hostgroup = -1;