Ok, agree.
Patch looks simple and good to me.
Thank you!
WBR,
Alexey.
On May 11, 2009, at 18:20 , Bongjae Chang wrote:
> Hi Alexey,
>
> I am sorry for wrong patch.
>
> The former patch could have a memory leak because connectionMap
> could keep the invalid ConnectionState.
>
> The former patch doesn't just call
> entry.idleConnections.offer( conn ) if conn is not connected.
>
> So it seems that the following patch is better.
>
>
> In OutboutConnectionCacheBlockingImpl.java
> -----
>
> private boolean reclaimOrClose( ConnectionState<C> cs, final C
> conn ) {
> ...
> try {
> ...
> final boolean isOverflow = (numberOfConnections() >
> highWaterMark()) || !isConnected( conn );
> if (isOverflow) {
> ...
> close( conn );
> }
> }
> ...
>
>
> }
> -----
>
> Thanks.
> --
> Bongjae Chang
>
>
> ----- Original Message -----
> From: Bongjae Chang
> To: dev_at_grizzly.dev.java.net
> Sent: Tuesday, May 12, 2009 12:45 AM
> Subject: Re: About controlling the failure connection in connection
> management cache
>
> Hi Alexey,
>
> Alexey wrote:
> >the issue you're writing about really exists, but not sure we can
> really solve it in appropriate way.
> >Connection could be closed at any time: before returning to pool,
> when stored in pool, after restored from pool.
>
>
> You are right. But I think most of problems can be resolved if
> cacheable pool prevents invalid connection from being returned and
> user tries to reconnect the server next.
>
> |
> |<------------ connection was closed (1)
> |
> |<- returning to pool
> |
> |<------------ connection was closed when stored in pool (2)
> |
> |<- after restored from pool
> |
> |<------------ connection was closed (3)
> |
>
> (1) pool can know whether the connection is valid or not. If the
> connection is not valid, pool doesn't return it to the cache. Then,
> next acquireConnectorHandler() and connect() can establish new
> connection because pool doesn't have the connection cache.
>
>
> (2) pool maybe returns the closed connection, so user may receive an
> exception when user calls connect(). But user releases the connector
> handler in finally phase like (*), then next
> acquireConnectorHandler() and connect() can establish new connection.
>
> Here is sample step.
>
> (a) the connection is closed when stored in pool
> (b) SelectionKeyHandler#cancel() is called
> (c) the channel is closed
> (d) when user calls connect() and channel#register(selector,
> SelectionKey.OP_CONNECT) is called in
> CacheableConnectorHandler_at_notifyCallbackHandlerPseudoConnect(), user
> receives ClosedChannelException.
> (e) when user releases the connector handler like (*), the connector
> doesn't return it to the cache.
>
>
> (3) user can receive an exception when user calls send(). But user
> releases the connector in finally phase like (*), then next
> acquireConnectorHandler() and connect() can establish new connection.
>
>
> I think that the following is a common user's code.
>
> try{
> ...
> connectorHandler = controller.acquireConnectorHandler(...);
> connectorHandler.connect(...);
> // send logic
> } catch(...) {
> ...
> } finally { ----------- (*)
> if( connectorHandler != null ) {
> connectorHandler.close();
> controller.releaseConnectorHandler( connectorHandler );
> }
> }
>
> The current problem is that user is not able to reconnect the remote
> server forever if the connection is closed anywhere because invalid
> connector is cached and returned again when user calls next
> acquireConnectorHandler() and connect().
>
> Alexey wrote:
> Currently we leave it up to developer to resolve this.
> But, if you have an idea, how we can solve this in general - will
> appreciate that.
>
>
> I agree that my thought that the pool should prevent a invalid
> connector from being returned maybe can not resolve this perfectly,
> but I think that at least user should be able to connect the server
> next when user tries to reconnect the server after the server revives.
>
> So I think that this could be the logic.
>
> >In OutboundConnectionCacheBlockingImpl.java,
> >-----
> >public synchronized void release( final C conn, final int
> numResponsesExpected ){
> > ...
> > final ConnectionState<C> cs = connectionMap.get( conn );
> > try {
> > if( cs == null ) {
> > ...
> > return;
> > } else {
> > ...
> > if(numBysy == 0 ) {
> > ...
> > if(isConnected( conn ) && !connectionClosed )
> > //if(!connectionClosed) {
> > ...
> > entry.idleConnections.offer( conn );
> > cs.csv = ConnectionStateValue.IDLE;
> > }
> > }
> > }
> > }
> > ...
> >}
>
> When I tested this patch, it seems that above (1), (2) and (3)'s
> case works well.
>
> Please advice me if my thought is wrong.
>
> Thanks.
>
> --
> Bongjae Chang
>
>
> ----- Original Message -----
> From: Oleksiy Stashok
> To: dev_at_grizzly.dev.java.net
> Sent: Monday, May 11, 2009 11:10 PM
> Subject: Re: About controlling the failure connection in connection
> management cache
>
> Hi,
>
> the issue you're writing about really exists, but not sure we can
> really solve it in appropriate way.
> Connection could be closed at any time: before returning to pool,
> when stored in pool, after restored from pool.
>
> Currently we leave it up to developer to resolve this.
> But, if you have an idea, how we can solve this in general - will
> appreciate that.
>
> Thank you.
>
> WBR,
> Alexey.
>
>
> On May 11, 2009, at 6:25 , Bongjae Chang wrote:
>
>> Hi,
>>
>> When I used the connection management cache with
>> CacheableConnectorHandlerPool, the connection which was once failed
>> couldn't reconnect the remote endpoint.
>>
>> It seems that the pool always stores the connector handler without
>> checking the validity.
>>
>> If you try to connect the invalid remote which was shut down for a
>> while, and if you try to reconnect the remote after it revives, you
>> can always see the following error.
>>
>> -----
>> java.nio.channels.ClosedChannelException
>> at
>> java
>> .nio
>> .channels
>> .spi
>> .AbstractSelectableChannel.register(AbstractSelectableChannel.java:1
>> 94)
>> at
>> java.nio.channels.SelectableChannel.register(SelectableChannel.java:
>> 277)
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client.CacheableConnectorHandler.notifyCallbackHandlerPse
>> udoConnect(CacheableConnectorHandler.java:199)
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client.CacheableConnectorHandler.doConnect(CacheableConne
>> ctorHandler.java:161)
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client.CacheableConnectorHandler.connect(CacheableConnect
>> orHandler.java:115)
>> at
>> SimpleTestServerForCache.sendTcpTest(SimpleTestServerForCache.java:
>> 154)
>> at SimpleTestServerForCache
>> $1.run(SimpleTestServerForCache.java:51)
>> at java.lang.Thread.run(Thread.java:717)
>> -----
>>
>> Then, maybe you can't connect the remote any longer regardless of
>> the remote's state.
>>
>> So, I think that the cacheable pool should try to check the
>> connector handler's validity when the connector handler is released
>> and the pool should not return the connector handler to idle
>> connections if the handler is not connected.
>>
>> Here is the sample.
>>
>> In OutboundConnectionCacheBlockingImpl.java,
>> -----
>> public synchronized void release( final C conn, final int
>> numResponsesExpected ){
>> ...
>> final ConnectionState<C> cs = connectionMap.get( conn );
>> try {
>> if( cs == null ) {
>> ...
>> return;
>> } else {
>> ...
>> if(numBysy == 0 ) {
>> ...
>> if(isConnected( conn ) && !connectionClosed )
>> //if(!connectionClosed) {
>> ...
>> entry.idleConnections.offer( conn );
>> cs.csv = ConnectionStateValue.IDLE;
>> }
>> }
>> }
>>
>> }
>> ...
>> }
>>
>> @SuppressWarnings( "unchecked" )
>> private boolean isConnected( final C conn ) {
>> if( conn instanceof TCPConnectorHandler ) {
>> return ((TCPConnectorHandler)conn).isConnected();
>> } else if ( conn instanceof UDPConnectorHandler ) {
>> return ((UDPConnectorHandler)conn).isConnected();
>> } else if ( conn instanceof SSLConnectorHandler ) {
>> return ((SSLConnectorHandler)conn).isConnected();
>> } else {
>> return true;
>> }
>> }
>> -----
>>
>> If ConnectorHandlers' isConnected() method is moved into higher
>> position like AbstractConnectorHandler class or ConnectorHandler
>> interface as I said from the former mail(https://grizzly.dev.java.net/servlets/ReadMsg?list=dev&msgNo=2591
>> ), the patch could perhaps be simpler.
>>
>> Thanks.
>>
>> --
>> Bongjae Chang
>
>