Merge branch 'v3.0-5473' into v3.0-5496

Signed-off-by: René Cannaò <rene@proxysql.com>

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

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;
+}

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

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

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();
+}