Milestone 3 Phase 3.5: Extract hostgroup routing and locking decisions from session #5529
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| #ifndef __MYSQL_HOSTGROUP_ROUTING_H | ||
| #define __MYSQL_HOSTGROUP_ROUTING_H | ||
|
|
||
| #include <string> | ||
|
|
||
| /** | ||
| * @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 | ||
| }; | ||
|
Comment on lines
+26
to
+32
There was a problem hiding this comment. In this extracted API, Suggested fix: rename this field to something semantics-based like |
||
|
|
||
| /** | ||
| * @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; | ||
| }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * @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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| #ifndef __PGSQL_HOSTGROUP_ROUTING_H | ||
| #define __PGSQL_HOSTGROUP_ROUTING_H | ||
|
There was a problem hiding this comment. The include guard uses a reserved identifier ( Suggested fix: rename the guard to a non-reserved, project-scoped macro (e.g., PROXYSQL_PGSQL_HOSTGROUP_ROUTING_H) and update the closing comment accordingly. |
||
|
|
||
| #include <string> | ||
|
|
||
| /** | ||
| * @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; | ||
| }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * @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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| #include "MySQL_HostGroup_Routing.h" | ||
|
|
||
| MySQL_Routing_Result resolve_hostgroup_routing( | ||
|
Check failure on line 3 in lib/MySQL_HostGroup_Routing.cpp
|
||
| 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; | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. The new pure routing function doesn’t reproduce the existing STATE_SLEEP hostgroup reset behavior. When set_query_lock_on_hostgroup==1 and qpo_state.destination_hostgroup<0, this function leaves new_current_hostgroup unchanged (and can even return an error if current_hostgroup != locked_on_hostgroup). In MySQL_Session::STATE_SLEEP the result is used to set current_hostgroup without checking res.error, so sessions can keep a stale hostgroup instead of resetting to default_hostgroup/locked_on_hostgroup as before. Suggested fix: add an explicit “sleep/reset” mode/parameter (or a dedicated helper) that implements the old STATE_SLEEP logic: if transaction_persistent_hostgroup==-1 then new_current_hostgroup = (locked_on_hostgroup>=0 ? locked_on_hostgroup : default_hostgroup) when locking is enabled, otherwise default_hostgroup. Use that at the STATE_SLEEP call site(s). |
||
| } 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; | ||
| } | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| return result; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include guard uses a reserved identifier (
__MYSQL_HOSTGROUP_ROUTING_H). Identifiers beginning with double underscores are reserved to the implementation and can cause undefined behavior or conflicts with system headers.Suggested fix: rename the guard to a non-reserved, project-scoped macro (e.g., PROXYSQL_MYSQL_HOSTGROUP_ROUTING_H) and update the closing comment accordingly.