From db40505d189e8d52c94ced289407dfa7990cd759 Mon Sep 17 00:00:00 2001 From: jesmarcannao Date: Mon, 16 Feb 2026 21:30:29 +0000 Subject: [PATCH] Fix #2233: Mirror sessions destination_hostgroup overwritten by fast routing Bug fix: - Mirror sessions had their destination_hostgroup incorrectly overwritten by fast routing rules, causing duplicate query execution to the same hostgroup - Added check in Query_Processor.cpp to skip fast routing for mirror sessions Changes: - lib/Query_Processor.cpp: Wrapped fast routing logic with mirror session check - test/tap/tests/reg_test_2233_mirror_fast_routing-t.cpp: New regression test - test/tap/groups/groups.json: Added test to default-g1 test group - test/tap/tests/Makefile: Added build rule for the new test (commented out) The fix ensures mirror sessions maintain their assigned destination_hostgroup and are not affected by fast routing rules. --- lib/Query_Processor.cpp | 39 ++-- test/tap/groups/groups.json | 1 + test/tap/tests/Makefile | 3 + .../reg_test_2233_mirror_fast_routing-t.cpp | 192 ++++++++++++++++++ 4 files changed, 217 insertions(+), 18 deletions(-) create mode 100644 test/tap/tests/reg_test_2233_mirror_fast_routing-t.cpp diff --git a/lib/Query_Processor.cpp b/lib/Query_Processor.cpp index b02258b97..2eb8cbf21 100644 --- a/lib/Query_Processor.cpp +++ b/lib/Query_Processor.cpp @@ -1631,25 +1631,28 @@ __internal_loop: __exit_process_mysql_query: if (qr == NULL || qr->apply == false) { - // now it is time to check mysql_query_rules_fast_routing - // it is only check if "apply" is not true - const char * u = sess->client_myds->myconn->userinfo->username; - const char * s = sess->client_myds->myconn->userinfo->schemaname; - - int dst_hg = -1; - - if (_thr_SQP_rules_fast_routing != nullptr) { - proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 7, "Searching thread-local 'rules_fast_routing' hashmap with: user='%s', schema='%s', and flagIN='%d'\n", u, s, flagIN); - dst_hg = search_rules_fast_routing_dest_hg(&_thr_SQP_rules_fast_routing, u, s, flagIN, false); - } else if (rules_fast_routing != nullptr) { - proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 7, "Searching global 'rules_fast_routing' hashmap with: user='%s', schema='%s', and flagIN='%d'\n", u, s, flagIN); - // NOTE: A pointer to the member 'this->rules_fast_routing' is required, since the value of the - // member could have changed before the function acquires the internal lock. See function doc. - dst_hg = search_rules_fast_routing_dest_hg(&this->rules_fast_routing, u, s, flagIN, true); - } + // Skip fast routing for mirror sessions - they already have their destination + if (sess->mirror == false) { + // now it is time to check mysql_query_rules_fast_routing + // it is only check if "apply" is not true + const char * u = sess->client_myds->myconn->userinfo->username; + const char * s = sess->client_myds->myconn->userinfo->schemaname; + + int dst_hg = -1; + + if (_thr_SQP_rules_fast_routing != nullptr) { + proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 7, "Searching thread-local 'rules_fast_routing' hashmap with: user='%s', schema='%s', and flagIN='%d'\n", u, s, flagIN); + dst_hg = search_rules_fast_routing_dest_hg(&_thr_SQP_rules_fast_routing, u, s, flagIN, false); + } else if (rules_fast_routing != nullptr) { + proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 7, "Searching global 'rules_fast_routing' hashmap with: user='%s', schema='%s', and flagIN='%d'\n", u, s, flagIN); + // NOTE: A pointer to the member 'this->rules_fast_routing' is required, since the value of the + // member could have changed before the function acquires the internal lock. See function doc. + dst_hg = search_rules_fast_routing_dest_hg(&this->rules_fast_routing, u, s, flagIN, true); + } - if (dst_hg != -1) { - ret->destination_hostgroup = dst_hg; + if (dst_hg != -1) { + ret->destination_hostgroup = dst_hg; + } } } diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index ae613a80c..d7371d4c2 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -83,6 +83,7 @@ "reg_test_3606-mysql_warnings-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "reg_test_3625-sqlite3_session_client_error_limit-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "reg_test_3690-admin_large_pkts-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], + "reg_test_2233_mirror_fast_routing-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "reg_test_5233_set_warning-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "reg_test_3765_ssl_pollout-t" : [ "default-g2","mysql-auto_increment_delay_multiplex=0-g2","mysql-multiplexing=false-g2","mysql-query_digests=0-g2","mysql-query_digests_keep_comment=1-g2" ], diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index 6bb5ff810..9ecaccba0 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -180,6 +180,9 @@ else $(CXX) -DEXCLUDE_TRACKING_VARIABLES $< ../tap/SQLite3_Server.cpp -I$(CLICKHOUSE_CPP_IDIR) $(IDIRS) $(LDIRS) -L$(CLICKHOUSE_CPP_LDIR) -L$(LZ4_LDIR) $(OPT) $(OBJ) $(MYLIBSJEMALLOC) $(MYLIBS) $(STATIC_LIBS) $(CLICKHOUSE_CPP_LDIR)/libclickhouse-cpp-lib.a $(CLICKHOUSE_CPP_PATH)/contrib/zstd/zstd/libzstdstatic.a $(LZ4_LDIR)/liblz4.a -lscram -lusual -Wl,--allow-multiple-definition -o $@ endif +#reg_test_2233_mirror_fast_routing-t: reg_test_2233_mirror_fast_routing-t.cpp $(TAP_LDIR)/libtap.so +# $(CXX) $< $(TAP_PATH)/SQLite3_Server.cpp -I$(CLICKHOUSE_CPP_IDIR) $(IDIRS) $(LDIRS) -L$(CLICKHOUSE_CPP_LDIR) -L$(LZ4_LDIR) $(OPT) $(OBJ) $(MYLIBSJEMALLOC) $(MYLIBS) $(STATIC_LIBS) $(CLICKHOUSE_CPP_LDIR)/libclickhouse-cpp-lib.a $(CLICKHOUSE_CPP_PATH)/contrib/zstd/zstd/libzstdstatic.a $(LZ4_LDIR)/liblz4.a -lscram -lusual -Wl,--allow-multiple-definition -o $@ + %-t: %-t.cpp $(TAP_LDIR)/libtap.so $(CXX) $< $(IDIRS) $(LDIRS) $(OPT) $(MYLIBS) $(STATIC_LIBS) -o $@ diff --git a/test/tap/tests/reg_test_2233_mirror_fast_routing-t.cpp b/test/tap/tests/reg_test_2233_mirror_fast_routing-t.cpp new file mode 100644 index 000000000..182aa8ddd --- /dev/null +++ b/test/tap/tests/reg_test_2233_mirror_fast_routing-t.cpp @@ -0,0 +1,192 @@ +/** + * @file reg_test_2233_mirror_fast_routing-t.cpp + * @brief Regression test for issue #2233: Mirror sessions incorrectly have their + * destination_hostgroup overwritten by fast routing rules. + * + * This test verifies that: + * 1. Original queries are routed to the hostgroup specified by fast routing rules + * 2. Mirror queries are routed to the mirror_hostgroup and NOT overwritten by fast routing + */ + +#include +#include +#include +#include + +#include +#include +#include "mysql.h" + +#include "tap.h" +#include "command_line.h" +#include "utils.h" +#include "MySQL_Session.h" +#include "proxysql_structs.h" + +// Stub for SQLite3_Server_session_handler - required by SQLite3_Server.cpp +// but not used in this test +void SQLite3_Server_session_handler(MySQL_Session* sess, void* _pa, PtrSize_t* pkt) { + // This test doesn't use SQLite3_Server, so this is just a stub +} + + +const int MIRROR_HOSTGROUP = 2; +const int FAST_ROUTING_HOSTGROUP = 1; +const int DEFAULT_HOSTGROUP = 0; + +std::vector setup_queries = { + "DELETE FROM mysql_servers WHERE hostgroup_id IN (0, 1, 2)", + "INSERT INTO mysql_servers (hostgroup_id, hostname, port) VALUES (0, '127.0.0.1', 13306)", + "INSERT INTO mysql_servers (hostgroup_id, hostname, port) VALUES (1, '127.0.0.1', 13306)", + "INSERT INTO mysql_servers (hostgroup_id, hostname, port) VALUES (2, '127.0.0.1', 13306)", + "LOAD MYSQL SERVERS TO RUNTIME", + "DELETE FROM mysql_query_rules", + "DELETE FROM mysql_query_rules_fast_routing", + "LOAD MYSQL QUERY RULES TO RUNTIME" +}; + +int run_queries(MYSQL* mysql, std::vector& queries) { + for (const auto& query : queries) { + diag("Running: %s", query.c_str()); + if (mysql_query(mysql, query.c_str())) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(mysql)); + return 1; + } + } + return 0; +} + +int main(int argc, char** argv) { + CommandLine cl; + + if (cl.getEnv()) { + diag("Failed to get the required environmental variables."); + return -1; + } + + plan(3); + + // Initialize admin connection + MYSQL* proxysql_admin = mysql_init(NULL); + if (!proxysql_admin) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_admin)); + return -1; + } + + if (!mysql_real_connect(proxysql_admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_admin)); + return -1; + } + + // Setup servers and clear rules + diag("Setting up test environment..."); + if (run_queries(proxysql_admin, setup_queries)) { + return exit_status(); + } + + // Create query rule with mirror_hostgroup=2, apply=0 (no mirror_flagOUT) + // This rule will mirror the query but not apply, allowing fast routing to be evaluated + std::string mirror_rule = "INSERT INTO mysql_query_rules (rule_id, active, username, match_digest, mirror_hostgroup, apply) " + "VALUES (1, 1, '" + std::string(cl.username) + "', '^SELECT.*test_mirror', " + + std::to_string(MIRROR_HOSTGROUP) + ", 0)"; + diag("Creating mirror query rule: %s", mirror_rule.c_str()); + if (mysql_query(proxysql_admin, mirror_rule.c_str())) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_admin)); + return exit_status(); + } + + // Create fast routing rule with destination_hostgroup=1 + std::string fast_routing_rule = "INSERT INTO mysql_query_rules_fast_routing " + "(username, schemaname, flagIN, destination_hostgroup, comment) " + "VALUES ('" + std::string(cl.username) + "', 'information_schema', 0, " + + std::to_string(FAST_ROUTING_HOSTGROUP) + ", 'test_fast_routing')"; + diag("Creating fast routing rule: %s", fast_routing_rule.c_str()); + if (mysql_query(proxysql_admin, fast_routing_rule.c_str())) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql_admin)); + return exit_status(); + } + + // Load rules to runtime + MYSQL_QUERY(proxysql_admin, "LOAD MYSQL QUERY RULES TO RUNTIME"); + + // Initialize client connection + MYSQL* proxysql = mysql_init(NULL); + if (!proxysql) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql)); + return -1; + } + + if (!mysql_real_connect(proxysql, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(proxysql)); + return exit_status(); + } + + // Clear stats + MYSQL_QUERY(proxysql_admin, "SELECT COUNT(*) FROM stats_mysql_query_digest_reset"); + mysql_free_result(mysql_store_result(proxysql_admin)); + + // Execute test query that matches both mirror and fast routing rules + // The query uses 'test_mirror' in the digest to match the mirror rule + diag("Executing test query that matches both mirror and fast routing rules..."); + std::string test_query = "SELECT /* test_mirror */ 1"; + MYSQL_QUERY(proxysql, test_query.c_str()); + MYSQL_RES* result = mysql_store_result(proxysql); + mysql_free_result(result); + + // Wait a bit for stats to be updated + sleep(1); + + // Check stats_mysql_query_digest to verify routing + // We expect: + // - Original query routed to FAST_ROUTING_HOSTGROUP (1) + // - Mirror query routed to MIRROR_HOSTGROUP (2) + std::string check_query = "SELECT hostgroup, COUNT(*) FROM stats_mysql_query_digest " + "WHERE digest_text LIKE '%test_mirror%' " + "GROUP BY hostgroup ORDER BY hostgroup"; + diag("Checking query routing: %s", check_query.c_str()); + MYSQL_QUERY(proxysql_admin, check_query.c_str()); + MYSQL_RES* stats_res = mysql_store_result(proxysql_admin); + + int num_rows = mysql_num_rows(stats_res); + diag("Number of distinct hostgroups used: %d", num_rows); + + // We expect queries in two different hostgroups (1 and 2) + ok(num_rows == 2, "Queries should be routed to 2 different hostgroups (original + mirror), found: %d", num_rows); + + if (num_rows == 2) { + MYSQL_ROW row; + int found_hostgroups[2] = {0, 0}; + int i = 0; + while ((row = mysql_fetch_row(stats_res)) && i < 2) { + int hostgroup = atoi(row[0]); + long count = atol(row[1]); + diag("Hostgroup %d: %ld queries", hostgroup, count); + found_hostgroups[i++] = hostgroup; + } + + // Verify the hostgroups are correct + bool has_fast_routing_hg = (found_hostgroups[0] == FAST_ROUTING_HOSTGROUP || found_hostgroups[1] == FAST_ROUTING_HOSTGROUP); + bool has_mirror_hg = (found_hostgroups[0] == MIRROR_HOSTGROUP || found_hostgroups[1] == MIRROR_HOSTGROUP); + + ok(has_fast_routing_hg, "Original query should be routed to fast routing hostgroup %d", FAST_ROUTING_HOSTGROUP); + ok(has_mirror_hg, "Mirror query should be routed to mirror hostgroup %d (NOT overwritten by fast routing)", MIRROR_HOSTGROUP); + } else { + // If we didn't get 2 hostgroups, fail the remaining tests + if (num_rows == 1) { + MYSQL_ROW row = mysql_fetch_row(stats_res); + int hostgroup = atoi(row[0]); + diag("ERROR: All queries went to single hostgroup %d", hostgroup); + diag("This indicates the bug is present: mirror query's destination_hostgroup was overwritten by fast routing!"); + } + ok(false, "Original query should be routed to fast routing hostgroup %d", FAST_ROUTING_HOSTGROUP); + ok(false, "Mirror query should be routed to mirror hostgroup %d", MIRROR_HOSTGROUP); + } + + mysql_free_result(stats_res); + + // Cleanup + mysql_close(proxysql); + mysql_close(proxysql_admin); + + return exit_status(); +}