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