One question: if I’m using thread-local connections ho does that affect pool sizing? Are thread-local connections included in the overall pool size or accounted for separately?
We may want some explicit release notes if a user would need to resize their pools during an upgrade. Anthony > On Apr 5, 2019, at 6:34 AM, Jacob Barrett <jbarr...@pivotal.io> wrote: > > Devs, > > The current connection pooling implementation contains a setting that enables > a secondary pool that is thread local. See ClientCacheFactory. > setPoolThreadLocalConnections method for details. This thread local pooling > was added to reduce contention on the primary connection pool under high > thread count or load. As of the completion of GEODE-6515 there is no longer a > contention issue in the primary pool under high thread count or load, > effectively rendering thread local pooling a no-op. Thread local pooling also > introduces many complexities into the primary pool management of connections > since a connection can either be in the primary pool or a pool for each > thread. Connection idle timeout, lifetime expiration and load conditioning > all have to work around thread local connections making for some interesting > relationships and behavior. Additionally clients of thread local connections > needed to call Pool.releaseThreadLocalConnections prior to the thread > terminating or connections would be held until the GC finalized the thread > local storage. Use of thread local connections typically mean significantly > high connection counts on the servers since each thread could horde > connections to each server regardless of current workload need. > > I propose that we deprecate all the public APIs for thread local connections > and immediately remove the behavior. The API methods will go from an > effective no-op to an actual no-op. A warning will be logged when > ClientCacheFactory. setPoolThreadLocalConnections is used. Internally all > thread local connection implementation will be removed. The net effect to the > client will be the same as promised with thread local connections, that > content on the primary pool is reduced for high thread count and load. > Additionally, the connection count to each server will be reduced since > threads won’t be holding connections to servers they are not actively > communicating with. All in all this will be a net-positive improvement for > the consuming client and the distributed system as a whole. Internally this > will be huge win as it removes complexity and opens a path to removing even > more complexity in idle connection timeouts, connection lifetime expiration, > and load conditioning. Please see GEODE-6594 for more details. > > I realize this is sort of a grey area. An obvious questions is, “doesn’t this > violate semver by removing a feature in a minor?” My answer is a solid, > “nope!” The feature is defined as “reducing contention on the connection > pool”, which is provided now by default in the refactored connection pool. > All that is being done is deprecating and warning that a useless API is being > removed in the next major release. The feature remains intact and client code > does not need to be re-compiled or changed. > > I would love to hear if anyone has any concerns, questions or dissenting > opinions about this proposal. PR 3394 has been opened for review of the code > changes related to the removal. Please provide feedback in the PR relating > only to the code and discussions about the merits of he proposal here in this > email thread. > > Thanks, > Jake > > GEODE-6515 - https://issues.apache.org/jira/browse/GEODE-6515 > <https://issues.apache.org/jira/browse/GEODE-6515> > GEODE-6594 - https://issues.apache.org/jira/browse/GEODE-6594 > <https://issues.apache.org/jira/browse/GEODE-6594> > PR 3394 - https://github.com/apache/geode/pull/3394 > <https://github.com/apache/geode/pull/3394> >
