Added support of goodbye message before closing the driver. #529
Conversation
Used ChannelGroup to keep track of all alive channels.
| MetricsListener metricsListener, Logging logging, Clock clock ) | ||
| { | ||
| this( connector, bootstrap, new NettyChannelTracker( metricsListener, logging ), settings, metricsListener, logging, clock ); | ||
| this( connector, bootstrap, new NettyChannelTracker( metricsListener, eventExecutorGroup, logging ), settings, metricsListener, logging, clock ); |
There was a problem hiding this comment.
I think EventExecutorGroup parameter can be removed. Channel tracker can be created using the given bootstrap:
new NettyChannelTracker( metricsListener, bootstrap.config().group().next(), logging )| @@ -159,6 +160,7 @@ public int idleConnections( BoltServerAddress address ) | |||
| ChannelPool pool = entry.getValue(); | |||
|
|
|||
| log.info( "Closing connection pool towards %s", address ); | |||
| nettyChannelTracker.destructAllChannels(); | |||
There was a problem hiding this comment.
We have a single channel tracker for all pools. This method is invoked once for every pool. Shouldn't it be called once outside of the outer loop?
| @@ -95,6 +108,8 @@ public void channelClosed( Channel channel ) | |||
| { | |||
| decrementIdle( channel ); | |||
| metricsListener.afterClosed( serverAddress( channel ) ); | |||
|
|
|||
| allChannels.remove( channel ); | |||
There was a problem hiding this comment.
Closed channels are automatically removed from a channel group. This line can be removed
| * Destruct channel before it is destroyed. | ||
| * @param channel the channel to destroy. | ||
| */ | ||
| void destructChannel( Channel channel ); |
| RECORD [1] | ||
| SUCCESS {"bookmark": "bookmark:1"} | ||
| C: GOODBYE | ||
| S: SUCCESS {} |
There was a problem hiding this comment.
Database does not send SUCCESS for GOODBYE. Better replace with S: <EXIT>
|
|
||
| public NettyChannelTracker( MetricsListener metricsListener, Logging logging ) | ||
| public NettyChannelTracker( MetricsListener metricsListener, EventExecutorGroup eventExecutorGroup, Logging logging ) |
There was a problem hiding this comment.
I think this can just take and EventExecutor and pass it directly to the DefaultChannelGroup constructor. bootstrap.config().group().next() returns an EventExecutor.
| catch ( Throwable e ) | ||
| { | ||
| // only logging it | ||
| log.debug( "Failed to destruct Channel %s due to error %s. " + |
There was a problem hiding this comment.
Maybe reword this a bit to remove "destruct"?
Used ChannelGroup to keep track of all alive channels.