From 2a7bddca28efa23ebf40e95d9654718bc10b65ee Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 22 Apr 2026 06:39:50 +0000 Subject: [PATCH] MySrvC: rename server_backoff_time to session_track_backoff_until, gate hot-path read Context ------- PR #5166 introduced a per-'MySrvC' atomic timestamp that prevents the server from being chosen for new connections for a 30-second window. The timestamp is written in exactly one place, 'MySQL_Session::handle_session_track_capabilities()', and only when 'mysql-session_track_variables' is in ENFORCED mode and the backend is missing 'CLIENT_SESSION_TRACKING' or 'CLIENT_DEPRECATE_EOF'. The field was originally named 'server_backoff_time' and its only semantics were that ENFORCED-mode capability mismatches temporarily exclude the server from selection. Two issues with the original shape: 1. The name does not hint that the scope is session-tracking capability enforcement. A reader of 'MyHGC::get_random_MySrvC' or 'MySQL_Thread::get_MyConn_local' had no way to tell that this field is tied to one specific feature and not, say, a generic health backoff. 2. The atomic load was unconditional in both hot selection loops. Even though the atomic is relaxed (effectively a 'mov' on x86), per-iteration atomic reads inhibit hoisting and CSE that the compiler can otherwise apply to thread-local integer reads. 'get_random_MySrvC' runs the loop over every server in the hostgroup on every backend-connection acquisition, so this is the sort of place where small per-iteration costs accumulate. Changes ------- * Rename the field: MySrvC::server_backoff_time -> MySrvC::session_track_backoff_until Naming it after the feature makes it obvious that the field is specific to session-tracking enforcement and should not be overloaded for unrelated purposes (e.g. connect errors, monitor failures). If a future change needs a generic backoff mechanism, a new field -- or a rename at that point -- keeps intent clear rather than piggy-backing on this one silently. * Gate the atomic load on mode: In both 'MyHGC::get_random_MySrvC()' (lib/MyHGC.cpp) and 'MySQL_Thread::get_MyConn_local()' (lib/MySQL_Thread.cpp), the relaxed load is skipped entirely when 'mysql_thread___session_track_variables' is not ENFORCED. In 'get_MyConn_local' the check is computed once before the loop; in 'get_random_MySrvC' the check is inside the loop but is a thread-local int read, which is free. Correctness note: switching out of ENFORCED at runtime makes any stale 'session_track_backoff_until' values in the pool immediately ignored, which is the desired behavior -- servers rejected while ENFORCED was active become eligible again the moment the operator relaxes the mode, matching how ProxySQL handles other runtime-variable-dependent state. No explicit reset is required. * Updated the field documentation in include/MySQL_HostGroups_Manager.h with the monotonic-timestamp unit, the list of sites that read/write it, the explicit scope (ENFORCED mode only), and the reset policy (no explicit reset; deadline ages out by comparison against curtime). No functional change for DISABLED / OPTIONAL configurations (the default for all installations). ENFORCED mode behaviour is unchanged. --- include/MySQL_HostGroups_Manager.h | 23 +++++++++++++++++------ lib/MyHGC.cpp | 19 ++++++++++++++----- lib/MySQL_Thread.cpp | 21 ++++++++++++++++----- lib/MySrvC.cpp | 2 +- 4 files changed, 48 insertions(+), 17 deletions(-) diff --git a/include/MySQL_HostGroups_Manager.h b/include/MySQL_HostGroups_Manager.h index 9f30f3bf9..e5055d21d 100644 --- a/include/MySQL_HostGroups_Manager.h +++ b/include/MySQL_HostGroups_Manager.h @@ -206,13 +206,24 @@ class MySrvC { // MySQL Server Container int32_t use_ssl; char *comment; - // 'server_backoff_time' stores a timestamp that prevents the server from being - // considered for random selection ('MyHGC::get_random_MySrvC') until that time passes. + // 'session_track_backoff_until' stores a monotonic timestamp (in microseconds, matching + // 'MySQL_Thread::curtime' / 'MySQL_Session::thread->curtime') that prevents the server + // from being considered for selection by 'MyHGC::get_random_MySrvC()' and + // 'MySQL_Thread::get_MyConn_local()' until 'curtime' exceeds it. // - // This is primarily used when `session_track_variables::ENFORCED` mode is active. - // If a server lacks the required capabilities in this mode, it is temporarily - // excluded from selection for a specified duration. - std::atomic server_backoff_time; + // Scope: populated exclusively by 'MySQL_Session::handle_session_track_capabilities()' + // when 'mysql-session_track_variables' is in ENFORCED mode and the backend lacks the + // 'CLIENT_SESSION_TRACKING' / 'CLIENT_DEPRECATE_EOF' capabilities required to honor the + // mode. The name is intentionally feature-specific: if future code needs to exclude + // servers for an unrelated reason, introduce a separate field or rename this one to a + // more generic identifier at that point rather than overloading it silently. + // + // Reset policy: no explicit reset path; the field ages out naturally once 'curtime' + // surpasses the stored deadline. Selection sites must read it via 'load()' and may + // elide the atomic read entirely when 'mysql-session_track_variables' is not ENFORCED + // (since only that mode ever writes the field), keeping the hot connection-selection + // path free of atomic loads in default configurations. + std::atomic session_track_backoff_until; MySrvConnList *ConnectionsUsed; MySrvConnList *ConnectionsFree; diff --git a/lib/MyHGC.cpp b/lib/MyHGC.cpp index c7ae3e203..3eee29dd9 100644 --- a/lib/MyHGC.cpp +++ b/lib/MyHGC.cpp @@ -18,7 +18,7 @@ MySrvC *MyHGC::get_random_MySrvC(char * gtid_uuid, uint64_t gtid_trxid, int max_ unsigned int TotalUsedConn=0; unsigned int l=mysrvs->cnt(); static time_t last_hg_log = 0; - unsigned long long server_backoff_time; + unsigned long long session_track_backoff_until; #ifdef TEST_AURORA unsigned long long a1 = array_mysrvc_total/10000; array_mysrvc_total += l; @@ -39,10 +39,19 @@ MySrvC *MyHGC::get_random_MySrvC(char * gtid_uuid, uint64_t gtid_trxid, int max_ for (j=0; jidx(j); if (mysrvc->get_status() == MYSQL_SERVER_STATUS_ONLINE) { // consider this server only if ONLINE - // skip servers that are in backoff period - server_backoff_time = mysrvc->server_backoff_time.load(std::memory_order_relaxed); - if (server_backoff_time > sess->thread->curtime) { - continue; + // Skip servers that are in a session-tracking capability backoff window. + // Only the ENFORCED mode ever writes 'session_track_backoff_until', so in + // DISABLED/OPTIONAL we elide the atomic read entirely: this keeps the hot + // server-selection loop free of per-iteration atomic loads in the common + // case. Flipping out of ENFORCED at runtime therefore also "forgets" any + // stale backoff deadlines immediately, which is the desired behavior + // (rejected servers become eligible again as soon as the operator relaxes + // the mode). + if (mysql_thread___session_track_variables == session_track_variables::ENFORCED) { + session_track_backoff_until = mysrvc->session_track_backoff_until.load(std::memory_order_relaxed); + if (session_track_backoff_until > sess->thread->curtime) { + continue; + } } if (mysrvc->myhgc->num_online_servers.load(std::memory_order_relaxed) <= mysrvc->myhgc->attributes.max_num_online_servers) { // number of online servers in HG is within configured range diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index 5c64354fe..277aac601 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -6389,16 +6389,27 @@ MySQL_Connection * MySQL_Thread::get_MyConn_local(unsigned int _hid, MySQL_Sessi if (sess->client_myds->myconn == NULL) return NULL; if (sess->client_myds->myconn->userinfo == NULL) return NULL; unsigned int i; - unsigned long long server_backoff_time; + unsigned long long session_track_backoff_until; + // Mirror of the hot-path optimization in 'MyHGC::get_random_MySrvC()': only ENFORCED + // mode ever writes 'MySrvC::session_track_backoff_until', so in DISABLED/OPTIONAL we + // can skip the per-iteration atomic load entirely. The common case stays allocation- + // and atomic-free; the branch is well-predicted because the mode changes rarely. + const bool check_session_track_backoff = + (mysql_thread___session_track_variables == session_track_variables::ENFORCED); std::vector parents; // this is a vector of srvers that needs to be excluded in case gtid_uuid is used MySQL_Connection *c=NULL; for (i=0; ilen; i++) { c = (MySQL_Connection *) cached_connections->index(i); - // skip servers that are in backoff period - server_backoff_time = c->parent->server_backoff_time.load(std::memory_order_relaxed); - if (server_backoff_time > curtime) { - continue; + // Skip cached connections whose parent server is inside the session-tracking + // capability backoff window. See 'MySrvC::session_track_backoff_until' for the + // full rationale; reads are relaxed because the deadline is compared against + // 'curtime' and small reordering is harmless. + if (check_session_track_backoff) { + session_track_backoff_until = c->parent->session_track_backoff_until.load(std::memory_order_relaxed); + if (session_track_backoff_until > curtime) { + continue; + } } if (c->parent->myhgc->hid==_hid && sess->client_myds->myconn->match_tracked_options(c)) { // options are all identical diff --git a/lib/MySrvC.cpp b/lib/MySrvC.cpp index 4f8cd2db9..df1fe0b8b 100644 --- a/lib/MySrvC.cpp +++ b/lib/MySrvC.cpp @@ -31,7 +31,7 @@ MySrvC::MySrvC( bytes_recv=0; max_connections_used=0; queries_gtid_sync=0; - server_backoff_time.store(0); + session_track_backoff_until.store(0); time_last_detected_error=0; connect_ERR_at_time_last_detected_error=0; shunned_automatic=false;