bneradt commented on code in PR #13447:
URL: https://github.com/apache/trafficserver/pull/13447#discussion_r4167233403
##########
src/proxy/http/HttpSessionManager.cc:
##########
@@ -420,21 +420,35 @@ HttpSessionManager::acquire_session(HttpSM *sm, sockaddr
const *ip, const char *
to_return = nullptr;
}
- // Otherwise, check the thread pool first
- if (this->get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_THREAD ||
- this->get_pool_type() == TS_SERVER_SESSION_SHARING_POOL_HYBRID) {
- retval = _acquire_session(ip, hostname_hash, sm, match_style,
TS_SERVER_SESSION_SHARING_POOL_THREAD);
- }
-
- // If you didn't get a match, and the global pool is an option go there.
+ // Always check the thread-local pool first. Multiplexing server sessions
+ // (HTTP/2) cannot be safely shared across threads -- their state is
+ // owned by the EThread that drives their connection -- so they are filed
+ // exclusively in the per-thread pool by `Http2ServerSession::add_session`.
+ // If the configured pool type is `global`
+ // or `global_locked`, only the global pool would otherwise be consulted,
+ // which means an existing H/2 origin connection on this thread is invisible
+ // to the lookup. Each new request would then open a fresh TCP+TLS+H/2
+ // handshake to the origin, defeating multiplexing entirely. Trying the
+ // thread-local pool first restores within-thread H/2 origin reuse without
+ // changing behavior for HTTP/1.x sessions, which fall through to the
+ // configured pool below on a thread-local miss or failed allocation.
+ // Multiplexing sessions remain in the thread pool even when allocation
fails.
+ retval = _acquire_session(ip, hostname_hash, sm, match_style,
TS_SERVER_SESSION_SHARING_POOL_THREAD);
+
+ bool const thread_retry = retval == HSMresult_t::RETRY;
if (retval != HSMresult_t::DONE) {
if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL == this->get_pool_type() ||
TS_SERVER_SESSION_SHARING_POOL_HYBRID == this->get_pool_type()) {
retval = _acquire_session(ip, hostname_hash, sm, match_style,
TS_SERVER_SESSION_SHARING_POOL_GLOBAL);
- } else if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED ==
this->get_pool_type())
+ } else if (TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED ==
this->get_pool_type()) {
retval = _acquire_session(ip, hostname_hash, sm, match_style,
TS_SERVER_SESSION_SHARING_POOL_GLOBAL_LOCKED);
+ }
}
+ if (thread_retry && this->get_pool_type() !=
TS_SERVER_SESSION_SHARING_POOL_THREAD) {
+ // The caller accounts for the global probe's result independently.
+ Metrics::Counter::increment(http_rsb.origin_reuse_fail);
+ }
return retval;
Review Comment:
Documented the per-probe semantics in the origin statistics guide, following
the option suggested in the latest human review: units are decisions, both
failed probes contribute, and global success or a miss can overlap with a
thread-probe failure. This preserves visibility of the thread probe. It
intentionally does not restore the final-acquisition-only semantics requested
here; leaving this thread open if a separate metric is still preferred. The
docs build passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
