You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2015/09/16 20:32:05 UTC

Review Request 38440: add thread pool for p2p reader threads and async close threads

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/
-----------------------------------------------------------

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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a
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

Diff: https://reviews.apache.org/r/38440/diff/

Testing
-------

precheckin

Thanks,

Darrel Schneider

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.


> On Sept. 16, 2015, 2:36 p.m., Bruce Schuchardt wrote:
> > Should there really be a default limit on the number of async-close threads?  Those threads can hang for a while if the machine on the other end crashes or there is a network problem.  That's why we have async-closing in the first place.  If the executor fills up with these hung threads nothing will be closed until they become unstuck.

But if we do have a problem causing close to take a long time do we want to "try" to create 30,000 threads to close them all? The large number of thread creations can result in OOM. I did wonder if we are having a problem with closes blocking for minutes if it might be limited to just a few of our peers. So perhaps we should have a thread pool dedicated to each other member instead of just one. What do you think of that idea?


- Darrel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99309
-----------------------------------------------------------


On Sept. 16, 2015, 11:32 a.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 11:32 a.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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.

If we have a network or machine issue that causes socket close to hang (or
take a very long time) on sockets to all other members then this change
solves a problem we had before. In the old code in this case we would
create at least one async socket closer thread for every socket we attempt
to close. Those threads would hang around as long as the socket close call
blocks.
Now we will have at most 8 threads per other member trying to close
sockets. We have seen the creation of a huge number of threads cause
problems.

If we have a network issue that causes socket close to hang (or take a very
long time) on sockets to just some other members but we still have normal
connectivity to some other members (a split brain situation) then the pool
per member allows us to still clean up sockets in a timely manner to the
healthy members. That is the benefit of have multiple pools instead of a
single one.

Let me know if I'm missing something that caused you to say we still have
the same problem.


On Wed, Sep 16, 2015 at 6:07 PM, Anilkumar Gingade <ag...@pivotal.io>
wrote:

> >> So perhaps we should have a thread pool dedicated to each other member
> instead of just one.
>
> We still have the same issue right? We just moved the problem to peer
> level pool....
>
> If there is network or machine issues, most likely this will be across all
> the peers and this may not be helpful...
>
> -Anil.
>
>
>
>
>
> On Wed, Sep 16, 2015 at 4:49 PM, Darrel Schneider <ds...@pivotal.io>
> wrote:
>
>>
>>
>> > On Sept. 16, 2015, 2:36 p.m., Bruce Schuchardt wrote:
>> > > Should there really be a default limit on the number of async-close
>> threads?  Those threads can hang for a while if the machine on the other
>> end crashes or there is a network problem.  That's why we have
>> async-closing in the first place.  If the executor fills up with these hung
>> threads nothing will be closed until they become unstuck.
>> >
>> > Darrel Schneider wrote:
>> >     But if we do have a problem causing close to take a long time do we
>> want to "try" to create 30,000 threads to close them all? The large number
>> of thread creations can result in OOM. I did wonder if we are having a
>> problem with closes blocking for minutes if it might be limited to just a
>> few of our peers. So perhaps we should have a thread pool dedicated to each
>> other member instead of just one. What do you think of that idea?
>> >
>> > Bruce Schuchardt wrote:
>> >     That seems like a good idea
>>
>> I added this and post an updated review
>>
>>
>> - Darrel
>>
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/38440/#review99309
>> -----------------------------------------------------------
>>
>>
>> On Sept. 16, 2015, 4:48 p.m., Darrel Schneider wrote:
>> >
>> > -----------------------------------------------------------
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/38440/
>> > -----------------------------------------------------------
>> >
>> > (Updated Sept. 16, 2015, 4:48 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/SocketCreator.java
>> ff4a22c08f909b731a3a70fa39a893cb5fc0015a
>> >
>>  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
>> >
>> > Diff: https://reviews.apache.org/r/38440/diff/
>> >
>> >
>> > Testing
>> > -------
>> >
>> > precheckin
>> >
>> >
>> > Thanks,
>> >
>> > Darrel Schneider
>> >
>> >
>>
>>
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Anilkumar Gingade <ag...@pivotal.io>.

>> So perhaps we should have a thread pool dedicated to each other member
instead of just one.

We still have the same issue right? We just moved the problem to peer level
pool....

If there is network or machine issues, most likely this will be across all
the peers and this may not be helpful...

-Anil.





On Wed, Sep 16, 2015 at 4:49 PM, Darrel Schneider <ds...@pivotal.io>
wrote:

>
>
> > On Sept. 16, 2015, 2:36 p.m., Bruce Schuchardt wrote:
> > > Should there really be a default limit on the number of async-close
> threads?  Those threads can hang for a while if the machine on the other
> end crashes or there is a network problem.  That's why we have
> async-closing in the first place.  If the executor fills up with these hung
> threads nothing will be closed until they become unstuck.
> >
> > Darrel Schneider wrote:
> >     But if we do have a problem causing close to take a long time do we
> want to "try" to create 30,000 threads to close them all? The large number
> of thread creations can result in OOM. I did wonder if we are having a
> problem with closes blocking for minutes if it might be limited to just a
> few of our peers. So perhaps we should have a thread pool dedicated to each
> other member instead of just one. What do you think of that idea?
> >
> > Bruce Schuchardt wrote:
> >     That seems like a good idea
>
> I added this and post an updated review
>
>
> - Darrel
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/#review99309
> -----------------------------------------------------------
>
>
> On Sept. 16, 2015, 4:48 p.m., Darrel Schneider wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/38440/
> > -----------------------------------------------------------
> >
> > (Updated Sept. 16, 2015, 4:48 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/SocketCreator.java
> ff4a22c08f909b731a3a70fa39a893cb5fc0015a
> >
>  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
> >
> > Diff: https://reviews.apache.org/r/38440/diff/
> >
> >
> > Testing
> > -------
> >
> > precheckin
> >
> >
> > Thanks,
> >
> > Darrel Schneider
> >
> >
>
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.


> On Sept. 16, 2015, 2:36 p.m., Bruce Schuchardt wrote:
> > Should there really be a default limit on the number of async-close threads?  Those threads can hang for a while if the machine on the other end crashes or there is a network problem.  That's why we have async-closing in the first place.  If the executor fills up with these hung threads nothing will be closed until they become unstuck.
> 
> Darrel Schneider wrote:
>     But if we do have a problem causing close to take a long time do we want to "try" to create 30,000 threads to close them all? The large number of thread creations can result in OOM. I did wonder if we are having a problem with closes blocking for minutes if it might be limited to just a few of our peers. So perhaps we should have a thread pool dedicated to each other member instead of just one. What do you think of that idea?
> 
> Bruce Schuchardt wrote:
>     That seems like a good idea

I added this and post an updated review


- Darrel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99309
-----------------------------------------------------------


On Sept. 16, 2015, 4:48 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 4:48 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Bruce Schuchardt <bs...@pivotal.io>.


> On Sept. 16, 2015, 9:36 p.m., Bruce Schuchardt wrote:
> > Should there really be a default limit on the number of async-close threads?  Those threads can hang for a while if the machine on the other end crashes or there is a network problem.  That's why we have async-closing in the first place.  If the executor fills up with these hung threads nothing will be closed until they become unstuck.
> 
> Darrel Schneider wrote:
>     But if we do have a problem causing close to take a long time do we want to "try" to create 30,000 threads to close them all? The large number of thread creations can result in OOM. I did wonder if we are having a problem with closes blocking for minutes if it might be limited to just a few of our peers. So perhaps we should have a thread pool dedicated to each other member instead of just one. What do you think of that idea?

That seems like a good idea


- Bruce


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99309
-----------------------------------------------------------


On Sept. 16, 2015, 6:32 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 6:32 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Bruce Schuchardt <bs...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99309
-----------------------------------------------------------


Should there really be a default limit on the number of async-close threads?  Those threads can hang for a while if the machine on the other end crashes or there is a network problem.  That's why we have async-closing in the first place.  If the executor fills up with these hung threads nothing will be closed until they become unstuck.

- Bruce Schuchardt


On Sept. 16, 2015, 6:32 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 6:32 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Bruce Schuchardt <bs...@pivotal.io>.


> On Sept. 17, 2015, 6:33 p.m., Bruce Schuchardt wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java, line 1241
> > <https://reviews.apache.org/r/38440/diff/2/?file=1075801#file1075801line1241>
> >
> >     should this use shutdown instead of shutdownNow?  Won't shutdownNow discard any unexecuted tasks that were just scheduled by the async-closing of connections during removeEndpoint processing?
> 
> Darrel Schneider wrote:
>     Yes, you are correct. My initial concern with shutdown was that it would block waiting for all those stuck closes to complete.
>     But both shutdown and shutdownNow do not wait for tasks already scheduled to finish. You need to call awaitTermination after you called shutdown if you want to wait for the tasks to complete. With both of these pools we don't want to wait for termination but we want the thread to go away once all the tasks that are already scheduled complete.
>     So I'll change this shutdownNow to shutdown.

with that change I think you're good to go


- Bruce


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99419
-----------------------------------------------------------


On Sept. 16, 2015, 11:48 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 11:48 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.


> On Sept. 17, 2015, 11:33 a.m., Bruce Schuchardt wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java, line 1241
> > <https://reviews.apache.org/r/38440/diff/2/?file=1075801#file1075801line1241>
> >
> >     should this use shutdown instead of shutdownNow?  Won't shutdownNow discard any unexecuted tasks that were just scheduled by the async-closing of connections during removeEndpoint processing?

Yes, you are correct. My initial concern with shutdown was that it would block waiting for all those stuck closes to complete.
But both shutdown and shutdownNow do not wait for tasks already scheduled to finish. You need to call awaitTermination after you called shutdown if you want to wait for the tasks to complete. With both of these pools we don't want to wait for termination but we want the thread to go away once all the tasks that are already scheduled complete.
So I'll change this shutdownNow to shutdown.


- Darrel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99419
-----------------------------------------------------------


On Sept. 16, 2015, 4:48 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 4:48 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Bruce Schuchardt <bs...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99419
-----------------------------------------------------------



gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java (line 1241)
<https://reviews.apache.org/r/38440/#comment156311>

    should this use shutdown instead of shutdownNow?  Won't shutdownNow discard any unexecuted tasks that were just scheduled by the async-closing of connections during removeEndpoint processing?


- Bruce Schuchardt


On Sept. 16, 2015, 11:48 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 11:48 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.

(Updated Sept. 22, 2015, 4:39 p.m.)

Review request for geode, Bruce Schuchardt and Kirk Lund.

Changes
-------

CacheClientProxy now always uses the SocketCloser on its socket. It now uses an AtomicBoolean to prevent scheduling it more than once.
The closer on CacheClientNotifier now waits 50ms for the asyncClose request to complete. The unit tests now configure a closer much like the one used by the notifier.
Connection no longer passes a Runnable to asyncClose. By doing this the Runnable kept the Connection instance alive. If you had a bunch of async closes queued up then this might consume more memory than is desired. The code that used to be in the Runnable is now in a new prepareForAsyncClose method. This code will not block for long periods of time like close can so it is ok to do it inline.

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.

Diffs (updated)
-----

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

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Barry Oglesby <bo...@pivotal.io>.

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

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.

(Updated Sept. 18, 2015, 2:43 p.m.)

Review request for geode, Bruce Schuchardt and Kirk Lund.

Changes
-------

Basically added unit tests. But here are some things I discovered from the unit test:
- changed SocketCloser.close to do shutdown instead of shutdownNow. This will not cause shutdown to block but does mean that any closes already scheduled will eventually be attempted.
- Connection now uses an atomic to make sure it only schedules one async close of its socket.
- When doing a non-async close (because the thread pool has been shutdown) the code now also runs the optionable "extra" runnable.
- Before we had core threads set to 1 on the pool and an unlimited queue. This caused us to never have more than one thread. Now core threads is set to max threads and allowCoreThreadTimeouts is set to true.

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.

Diffs (updated)
-----

Diff: https://reviews.apache.org/r/38440/diff/

Testing
-------

precheckin

Thanks,

Darrel Schneider

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.

(Updated Sept. 17, 2015, 4:47 p.m.)

Review request for geode, Bruce Schuchardt and Kirk Lund.

Changes
-------

Kirk had pointed out that the async close code did not really make sense in the SocketCreator class.
So I split it out into the new SocketCloser. This should make it easier to unit test.
Also now both CacheClientNotifier and ConnectionTable have their own SocketCloser instead of trying to share a static one.
This cleans up the close logic and allows the CacheClientNotifier to have smaller thread pools.

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.

Diffs (updated)
-----

Diff: https://reviews.apache.org/r/38440/diff/

Testing
-------

precheckin

Thanks,

Darrel Schneider

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.

(Updated Sept. 16, 2015, 4:48 p.m.)

Review request for geode, Bruce Schuchardt and Kirk Lund.

Changes
-------

A HashMap of ThreadPoolExecutors is now kept for the async close threads. They key is the string "who" that was already passed to asyncClose.
Now when a member goes away or when a CacheClientProxy closes it will also free up its async close threads.
I changed the default for the max number of async close threads per member from 32 to 8.

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.

Diffs (updated)
-----

gemfire-core/src/main/java/com/gemstone/gemfire/internal/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a
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

Diff: https://reviews.apache.org/r/38440/diff/

Testing
-------

precheckin

Thanks,

Darrel Schneider

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by Darrel Schneider <ds...@pivotal.io>.


> On Sept. 16, 2015, 2:36 p.m., anilkumar gingade wrote:
> > Do we need any new unit tests...

I added a unit tests


- Darrel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99310
-----------------------------------------------------------


On Sept. 18, 2015, 2: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, 2: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
> 
>

Re: Review Request 38440: add thread pool for p2p reader threads and async close threads

Posted by anilkumar gingade <ag...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38440/#review99310
-----------------------------------------------------------


Do we need any new unit tests...

- anilkumar gingade


On Sept. 16, 2015, 6:32 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38440/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2015, 6:32 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/SocketCreator.java ff4a22c08f909b731a3a70fa39a893cb5fc0015a 
>   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 
> 
> Diff: https://reviews.apache.org/r/38440/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>