[GitHub] [trafficserver] zwoop commented on a change in pull request #6978: Add option for hybrid global and thread session pools

[GitBox <gi...@apache.org>]

zwoop commented on a change in pull request #6978:
URL: https://github.com/apache/trafficserver/pull/6978#discussion_r502048357



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -990,6 +990,22 @@ mptcp
    ``thread`` Re-use sessions from a per-thread pool.
    ========== =================================================================
 
+.. ts:cv:: CONFIG proxy.config.http.server_session_sharing.pool_hybrid_limit INT 0
+
+   Setting :ts:cv:`proxy.config.http.server_session_sharing.pool` to global can reduce
+   the number of connections to origin for some traffic loads.  However, if many
+   execute threads are active, the thread contention on the global pool can reduce the
+   lifetime of connections to origin and reduce effective origin connection reuse. The
+   :ts:cv:`proxy.config.http.server_session_sharing.pool_hybrid_limit` setting lets you
+   enable a hybrid pool mode using both global and per thread pools.  If hybrid mode
+   is enabled, sessons are returned to the local thread pool if the global pool lock is
+   not acquired rather than just closing the origin connection as is the case in standard
+   global mode.
+
+   Setting it to 0, disables hybrid mode.  Setting it to -1, enables hybrid

Review comment:
       Agreed. Adding a setting which there is no way to find a better value for than "on or off" doesn't make sense, and does more harm. I'd say nuke this setting for now (until it's shown to have a need), and add this hybrid model to the existing setting with another numeric value.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org