----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38440/#review100018 -----------------------------------------------------------
I looked at the CacheClientProxy changes. I just have a few questions/comments. CacheClientProxy uses asyncClose in terminateDispatching but closeTransientFields (called by terminateDispatching in its finally) uses socket.close. The old SocketCreator.asyncClose call waited 50ms. The new SocketCloser.asyncClose call doesn't wait at all. This might cause the socket to be actually closed in closeTransientFields instead of terminateDispatching. I don't think this is a big deal, but its slightly different behavior. I wanted to verify that the 'this._remoteHostAddress != null' check in CacheClientProxy closeTransientFields is there because closeTransientFields could have been called more than once for the same CacheClientProxy. - Barry Oglesby On Sept. 18, 2015, 9:43 p.m., Darrel Schneider wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38440/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2015, 9:43 p.m.) > > > Review request for geode, Bruce Schuchardt and Kirk Lund. > > > Bugs: GEODE-332 > https://issues.apache.org/jira/browse/GEODE-332 > > > Repository: geode > > > Description > ------- > > The changes in SocketCreator.java are for pooling of async close > (ConnectionTable also calls closeAsyncThreadExecutor). > > The changes in Connection.java and ConnectionTable.java are for pooling of > p2p reader threads. > > The async close thread pool will use at most 32 threads to do socket closes. > Once all 32 threads are busy the unlimited LinkedBlockingQueue starts queuing > close requests. The number of threads can be changed from 32 by using the > "p2p.ASYNC_CLOSE_POOL_MAX_THREADS" system property. > If a thread in this pool is not used for 120 seconds it will timeout (this > timeout can be changed using the "p2p.ASYNC_CLOSE_POOL_KEEP_ALIVE_TIME" > system property). > The threads requesting a socket close will not wait for the close to > complete. The previous code (that created a thread for every request) waited > 50ms for the request to complete. This can be enabled using the > "p2p.ASYNC_CLOSE_WAIT_MILLISECONDS" system property. > > The p2p reader thread pool has an unlimited number of threads. The pool is > used anytime a peer connects to us to create a p2p reader for his sender. It > is also used on the sender side to do the initial handshake when connecting. > If a thread in this pool is not used for 120 seconds it will timeout (this > timeout can be changed using the "p2p.READER_POOL_KEEP_ALIVE_TIME" system > property). > > > Diffs > ----- > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCloser.java > PRE-CREATION > gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java > ff4a22c08f909b731a3a70fa39a893cb5fc0015a > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientNotifier.java > 2cede256f4fd26e46478c459ea5c7ada00161f2f > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java > 15f83bb344e453caaad6269ffd63a28f166a02b6 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/Connection.java > cd1b7dc998df343a1e035fb9cb68f071073d362c > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/tcp/ConnectionTable.java > 9beb947dfc130634f60022d7385c24e985acab4b > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/SocketCloserJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/SocketCloserWithWaitJUnitTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/38440/diff/ > > > Testing > ------- > > precheckin > > > Thanks, > > Darrel Schneider > >
