-
Notifications
You must be signed in to change notification settings - Fork 157
Move error handling logic to the connection layer #322
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
Conversation
It was previously spread between `RoutingSession`, `RoutingTransaction` and `RoutingStatementResult` and they all did the same thing when connection errors happened. This commit introduces `RoutingPooledConnection` that handles errors for routing driver by removing entries from the routing table and purging connection pool for failed address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
| // In the future, we might be able to move this logic to the server. | ||
| switch ( accessMode ) | ||
| { | ||
| case READ: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually do not need this READ/WRITE mode as the error itself already self explained:
Neo.ClientError.Cluster.NotALeader -> server at xx no longer accepts writes
Neo.ClientError.General.ForbiddenOnReadOnlyDatabase -> Write queries cannot be performed in READ access mode
So we could totally remove access mode from this class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to get NotALeader when trying to write in a read session with connection towards follower and ForbiddenOnReadOnlyDatabase when connection is towards read replica. So it looks like we still need this switch on the access mode.
|
|
||
| public interface PooledConnection extends Connection | ||
| { | ||
| long lastUsedTimestamp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also move
onError
hasUnrecoverableErrors
from Connection to PooledConnection
|
|
||
| private static boolean isFailureToWrite( ClientException e ) | ||
| { | ||
| return e.code().equals( "Neo.ClientError.Cluster.NotALeader" ) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warning: you might get ClientException without code field at this stage :)
Methods `#onError()` and `#hasUnrecoverableErrors()` are only useful in pooled connection and their implementations threw exceptions in regular connection. This commit moves them to `PooledConnection` interface.
|
@zhenlineo comments should be addressed now |
It was previously spread between
RoutingSession,RoutingTransactionandRoutingStatementResultand they all did the same thing when connection errors happened.This PR introduces
RoutingPooledConnectionthat handles errors for routing driver by removing entries from the routing table and purging connection pool for failed address.