diff --git a/include/HostgroupRouting.h b/include/HostgroupRouting.h new file mode 100644 index 000000000..9ba9e386f --- /dev/null +++ b/include/HostgroupRouting.h @@ -0,0 +1,52 @@ +/** + * @file HostgroupRouting.h + * @brief Pure hostgroup routing decision logic for unit testability. + * + * Extracted from MySQL_Session::get_pkts_from_client() and + * PgSQL_Session::get_pkts_from_client(). The logic is identical + * for both protocols. + * + * @see Phase 3.5 (GitHub issue #5493) + */ + +#ifndef HOSTGROUP_ROUTING_H +#define HOSTGROUP_ROUTING_H + +/** + * @brief Result of a hostgroup routing decision. + */ +struct HostgroupRoutingDecision { + int target_hostgroup; ///< Resolved hostgroup to route to. + int new_locked_on_hostgroup; ///< Updated lock state (-1 = unlocked). + bool error; ///< True if an illegal HG switch was attempted. +}; + +/** + * @brief Resolve the target hostgroup given session state and QP output. + * + * Decision logic (mirrors get_pkts_from_client()): + * 1. Start with default_hostgroup as the target + * 2. If QP provides a destination (>= 0) and no transaction lock, + * use the QP destination + * 3. If transaction_persistent_hostgroup >= 0, override with transaction HG + * 4. If locking is enabled and lock_hostgroup flag is set, acquire lock + * 5. If already locked, verify target matches lock (error if mismatch) + * + * @param default_hostgroup Session's default hostgroup. + * @param qpo_destination_hostgroup Query Processor output destination (-1 = no override). + * @param transaction_persistent_hostgroup Current transaction HG (-1 = none). + * @param locked_on_hostgroup Current lock state (-1 = unlocked). + * @param lock_hostgroup_flag Whether the QP wants to acquire a lock. + * @param lock_enabled Whether set_query_lock_on_hostgroup is enabled. + * @return HostgroupRoutingDecision with resolved target and updated lock. + */ +HostgroupRoutingDecision resolve_hostgroup_routing( + int default_hostgroup, + int qpo_destination_hostgroup, + int transaction_persistent_hostgroup, + int locked_on_hostgroup, + bool lock_hostgroup_flag, + bool lock_enabled +); + +#endif // HOSTGROUP_ROUTING_H diff --git a/lib/HostgroupRouting.cpp b/lib/HostgroupRouting.cpp new file mode 100644 index 000000000..0c2d0d8d5 --- /dev/null +++ b/lib/HostgroupRouting.cpp @@ -0,0 +1,60 @@ +/** + * @file HostgroupRouting.cpp + * @brief Implementation of pure hostgroup routing decision logic. + * + * Mirrors the routing block in get_pkts_from_client() (MySQL_Session.cpp + * ~lines 5340-5377 and PgSQL_Session.cpp ~lines 2154-2189). + * + * @see HostgroupRouting.h + * @see Phase 3.5 (GitHub issue #5493) + */ + +#include "HostgroupRouting.h" + +HostgroupRoutingDecision resolve_hostgroup_routing( + int default_hostgroup, + int qpo_destination_hostgroup, + int transaction_persistent_hostgroup, + int locked_on_hostgroup, + bool lock_hostgroup_flag, + bool lock_enabled) +{ + HostgroupRoutingDecision d; + d.error = false; + d.new_locked_on_hostgroup = locked_on_hostgroup; + + // Start with default hostgroup + int current_hostgroup = default_hostgroup; + + // If QP provides a valid destination and no transaction lock, use it + if (qpo_destination_hostgroup >= 0 + && transaction_persistent_hostgroup == -1) { + current_hostgroup = qpo_destination_hostgroup; + } + + // Transaction affinity overrides everything + if (transaction_persistent_hostgroup >= 0) { + current_hostgroup = transaction_persistent_hostgroup; + } + + // Hostgroup locking logic (algorithm introduced in 2.0.6) + if (lock_enabled) { + if (locked_on_hostgroup < 0) { + // Not yet locked + if (lock_hostgroup_flag) { + // Acquire lock on the current (already resolved) hostgroup + d.new_locked_on_hostgroup = current_hostgroup; + } + } + if (d.new_locked_on_hostgroup >= 0) { + // Already locked (or just acquired) — enforce + if (current_hostgroup != d.new_locked_on_hostgroup) { + d.error = true; + } + current_hostgroup = d.new_locked_on_hostgroup; + } + } + + d.target_hostgroup = current_hostgroup; + return d; +} diff --git a/lib/Makefile b/lib/Makefile index c91a22854..7af5cd6ef 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -107,6 +107,7 @@ _OBJ_CXX := ProxySQL_GloVars.oo network.oo debug.oo configfile.oo Query_Cache.oo pgsql_tokenizer.oo \ MonitorHealthDecision.oo \ TransactionState.oo \ + HostgroupRouting.oo \ proxy_sqlite3_symbols.oo # TSDB object files diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index ee3de4ae2..999f3c30c 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -234,6 +234,7 @@ $(ODIR)/test_init.o: $(TEST_HELPERS_DIR)/test_init.cpp | $(ODIR) 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 \ + hostgroup_routing_unit-t \ transaction_state_unit-t .PHONY: all diff --git a/test/tap/tests/unit/hostgroup_routing_unit-t.cpp b/test/tap/tests/unit/hostgroup_routing_unit-t.cpp new file mode 100644 index 000000000..1d07bd1a3 --- /dev/null +++ b/test/tap/tests/unit/hostgroup_routing_unit-t.cpp @@ -0,0 +1,110 @@ +/** + * @file hostgroup_routing_unit-t.cpp + * @brief Unit tests for hostgroup routing decision logic. + * + * @see Phase 3.5 (GitHub issue #5493) + */ + +#include "tap.h" +#include "test_globals.h" +#include "test_init.h" +#include "proxysql.h" +#include "HostgroupRouting.h" + +static void test_basic_routing() { + // No transaction, no lock → uses QP destination + auto d = resolve_hostgroup_routing(0, 5, -1, -1, false, false); + ok(d.target_hostgroup == 5, "basic: QP destination used"); + ok(d.error == false, "basic: no error"); + + // QP destination -1 → uses default + auto d2 = resolve_hostgroup_routing(0, -1, -1, -1, false, false); + ok(d2.target_hostgroup == 0, "basic: default HG when QP=-1"); +} + +static void test_transaction_affinity() { + // Transaction active → overrides QP destination + auto d = resolve_hostgroup_routing(0, 5, 3, -1, false, false); + ok(d.target_hostgroup == 3, + "txn: transaction_persistent_hostgroup overrides QP"); + + // Transaction + QP both set → transaction wins + auto d2 = resolve_hostgroup_routing(0, 10, 7, -1, false, false); + ok(d2.target_hostgroup == 7, "txn: transaction wins over QP"); +} + +static void test_locking_acquire() { + // Lock enabled, lock_hostgroup=true, not yet locked → acquires lock + auto d = resolve_hostgroup_routing(0, 5, -1, -1, true, true); + ok(d.target_hostgroup == 5, "lock acquire: routes to QP dest"); + ok(d.new_locked_on_hostgroup == 5, + "lock acquire: lock set to target HG"); + ok(d.error == false, "lock acquire: no error"); +} + +static void test_locking_enforce() { + // Already locked on HG 5, QP routes to 5 → ok + auto d = resolve_hostgroup_routing(0, 5, -1, 5, false, true); + ok(d.target_hostgroup == 5, "lock enforce: same HG ok"); + ok(d.error == false, "lock enforce: no error on match"); + + // Already locked on HG 5, QP routes to 10 → error + auto d2 = resolve_hostgroup_routing(0, 10, -1, 5, false, true); + ok(d2.error == true, "lock enforce: error on HG mismatch"); + ok(d2.target_hostgroup == 5, + "lock enforce: stays on locked HG despite QP mismatch"); +} + +static void test_locking_disabled() { + // Lock feature disabled → no locking even with flag + auto d = resolve_hostgroup_routing(0, 5, -1, -1, true, false); + ok(d.new_locked_on_hostgroup == -1, + "lock disabled: no lock acquired"); + + // Lock feature disabled → no enforcement even if locked state passed + auto d2 = resolve_hostgroup_routing(0, 10, -1, 5, false, false); + ok(d2.target_hostgroup == 10, + "lock disabled: routes to QP dest ignoring lock"); + ok(d2.error == false, "lock disabled: no error"); +} + +static void test_transaction_plus_lock() { + // Transaction active + locked on same HG → no error + auto d = resolve_hostgroup_routing(0, 10, 3, 3, false, true); + ok(d.target_hostgroup == 3, + "txn+lock: transaction HG used"); + ok(d.error == false, "txn+lock: no error when txn matches lock"); + + // Transaction active on HG 3 but locked on HG 5 → error (mismatch) + auto d2 = resolve_hostgroup_routing(0, 10, 3, 5, false, true); + ok(d2.error == true, + "txn+lock: error when txn HG differs from lock HG"); +} + +static void test_edge_cases() { + // default_hostgroup=-1 (shouldn't happen but handle gracefully) + auto d = resolve_hostgroup_routing(-1, -1, -1, -1, false, false); + ok(d.target_hostgroup == -1, "edge: negative defaults pass through"); + + // All zeros + auto d2 = resolve_hostgroup_routing(0, 0, -1, -1, false, false); + ok(d2.target_hostgroup == 0, "edge: HG 0 is valid"); +} + +int main() { + plan(21); + int rc = test_init_minimal(); + ok(rc == 0, "test_init_minimal() succeeds"); + + test_basic_routing(); // 3 + test_transaction_affinity(); // 2 + test_locking_acquire(); // 3 + test_locking_enforce(); // 4 + test_locking_disabled(); // 3 + test_transaction_plus_lock(); // 3 + test_edge_cases(); // 2 + // Total: 1+3+2+3+4+3+3+2 = 21 + + test_cleanup_minimal(); + return exit_status(); +}