Skip to content
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

Relax connection termination policy in routing driver #436

Merged
merged 3 commits into from
Nov 29, 2017

Conversation

lutovich
Copy link
Contributor

Previously routing driver terminated all connections towards a particular address when one of active connections had a network error. Connections were also terminated when new routing table did not contain some address that was present in the previous routing table. Such behaviour might be problematic because it results in terminated queries. Network errors might have been temporary but always resulted in termination of active queries.

This PR makes driver keep connections towards machine that had failure but remove it from the routing table. This is done to prevent subsequent queries from using it until rediscovery. After rediscovery address can either appear again in the routing procedure response or not. If it is returned then driver will re-add it to routing table and old pool with all connections will be reused. If address is missing from routing table and pool has no active connections then driver will close the pool.

Previously routing driver terminated all connections towards a
particular address when one of active connections had a network error.
Connections were also terminated when new routing table did not contain
some address that was present in the previous routing table. Such
behaviour might be problematic because it results in terminated queries.
Network errors might have been temporary but always resulted in
termination of active queries.

This commit makes driver keep connections towards machine that had
failure but remove it from the routing table. This is done to prevent
subsequent queries from using it until rediscovery. After rediscovery
address can either appear again in the routing procedure response or not.
If it is returned then driver will re-add it to routing table and old
pool with all connections will be reused. If address is missing from
routing table and pool has no active connections then driver will close
the pool.
Methods `#purge(BoltServerAddress)` and `#hasAddress(BoltServerAddress)`
were only used in tests. Now they are gone.
@lutovich lutovich requested a review from zhenlineo November 27, 2017 09:49
if ( !addressesToRetain.contains( address ) )
{
int activeChannels = activeChannelTracker.activeChannelCount( address );
if ( activeChannels == 0 )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we also purge all IDLE connection if the activeChannel is not 0?
If so then maybe also a test: shouldCloseIdleConnInPoolWhenRetaining?

FYI:

Driver remove from RT close idle disallow return to pool
JS * o * o * o
python * o * o X X
.NET * o * o * o
Java 1.4 * o * o * o
Java 1.5 * o X X X X

[*]connection error
[o]not in new routing table

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it will not purge idle connections in that case. Whole pool will be closed when activeChannels == 0 and address is not in the routing table.

@zhenlineo zhenlineo merged commit 0c57215 into neo4j:1.5 Nov 29, 2017
@lutovich lutovich deleted the 1.5-no-purge branch November 29, 2017 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants