Skip to content

Prevent graceful connection pool shutdowns from logging connection errors #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,20 @@ public void channelInactive( ChannelHandlerContext ctx )
{
log.debug( "Channel is inactive" );

String terminationReason = terminationReason( ctx.channel() );
Throwable error = ErrorUtil.newConnectionTerminatedError( terminationReason );

if ( !failed )
{
// channel became inactive not because of a fatal exception that came from exceptionCaught
// it is most likely inactive because actual network connection broke or was explicitly closed by the driver

String terminationReason = terminationReason( ctx.channel() );
ServiceUnavailableException error = ErrorUtil.newConnectionTerminatedError( terminationReason );
fail( ctx, error );
messageDispatcher.handleChannelInactive( error );
ctx.channel().close();
}
else
{
fail( error );
}
}

Expand All @@ -89,16 +95,14 @@ public void exceptionCaught( ChannelHandlerContext ctx, Throwable error )
{
failed = true;
log.warn( "Fatal error occurred in the pipeline", error );
fail( ctx, error );
fail( error );
}
}

private void fail( ChannelHandlerContext ctx, Throwable error )
private void fail( Throwable error )
{
Throwable cause = transformError( error );
messageDispatcher.handleChannelError( cause );
log.debug( "Closing channel because of a failure '%s'", error );
ctx.close();
}

private static Throwable transformError( Throwable error )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Map;
import java.util.Queue;

import org.neo4j.driver.exceptions.ServiceUnavailableException;
import org.neo4j.driver.internal.handlers.ResetResponseHandler;
import org.neo4j.driver.internal.logging.ChannelActivityLogger;
import org.neo4j.driver.internal.messaging.ResponseMessageHandler;
Expand All @@ -45,6 +46,7 @@ public class InboundMessageDispatcher implements ResponseMessageHandler
private final Queue<ResponseHandler> handlers = new LinkedList<>();
private final Logger log;

private volatile boolean gracefullyClosed;
private Throwable currentError;
private boolean fatalErrorOccurred;

Expand Down Expand Up @@ -142,6 +144,20 @@ public void handleIgnoredMessage()
handler.onFailure( error );
}

public void handleChannelInactive( Throwable cause )
{
// report issue if the connection has not been terminated as a result of a graceful shutdown request from its
// parent pool
if ( !gracefullyClosed )
{
handleChannelError( cause );
}
else
{
channel.close();
}
}

public void handleChannelError( Throwable error )
{
if ( currentError != null )
Expand All @@ -160,6 +176,9 @@ public void handleChannelError( Throwable error )
ResponseHandler handler = removeHandler();
handler.onFailure( currentError );
}

log.debug( "Closing channel because of a failure '%s'", error );
channel.close();
}

public void clearCurrentError()
Expand All @@ -177,6 +196,11 @@ public boolean fatalErrorOccurred()
return fatalErrorOccurred;
}

public void prepareToCloseChannel( )
{
this.gracefullyClosed = true;
}

/**
* <b>Visible for testing</b>
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.neo4j.driver.internal.BookmarkHolder;
import org.neo4j.driver.internal.DatabaseName;
import org.neo4j.driver.internal.async.UnmanagedTransaction;
import org.neo4j.driver.internal.async.inbound.InboundMessageDispatcher;
import org.neo4j.driver.internal.cluster.RoutingContext;
import org.neo4j.driver.internal.cursor.AsyncResultCursorOnlyFactory;
import org.neo4j.driver.internal.cursor.ResultCursorFactory;
Expand Down Expand Up @@ -101,9 +102,13 @@ public void initializeChannel( String userAgent, AuthToken authToken, RoutingCon
@Override
public void prepareToCloseChannel( Channel channel )
{
InboundMessageDispatcher messageDispatcher = messageDispatcher( channel );

GoodbyeMessage message = GoodbyeMessage.GOODBYE;
messageDispatcher( channel ).enqueue( NoOpResponseHandler.INSTANCE );
messageDispatcher.enqueue( NoOpResponseHandler.INSTANCE );
channel.writeAndFlush( message, channel.voidPromise() );

messageDispatcher.prepareToCloseChannel( );
}

@Override
Expand Down