-
Notifications
You must be signed in to change notification settings - Fork 155
Multi-databases support for routing driver #587
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
925840e
to
2ffd5f4
Compare
a7a9a8d
to
a1ba224
Compare
…abase. As this makes the routing table map easier for server version lower than 4.0. Otherwise, if we default to get routing table for system_db, we need to detect if it is server version lower than 4.0. If so, we can only get routing table for default_db, and then we need to reset the key in routing table map to be default_db. TODO: Removing stale routing table from routing table map. Also missing any integration tests for 4.0 routing driver.
c567641
to
771e6d6
Compare
771e6d6
to
63d6f93
Compare
0de7d22
to
d09054a
Compare
…getRoutingTable`, as this procedure is not only used by cluster but also single instance.
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.
@zhenlineo Looks good to me. I've made small comments mostly with naming suggestions.
driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTableHandler.java
Outdated
Show resolved
Hide resolved
driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTableRegistryImpl.java
Outdated
Show resolved
Hide resolved
driver/src/main/java/org/neo4j/driver/exceptions/RoutingException.java
Outdated
Show resolved
Hide resolved
driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java
Outdated
Show resolved
Hide resolved
driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunner.java
Outdated
Show resolved
Hide resolved
driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingProcedureRunner.java
Outdated
Show resolved
Hide resolved
{ | ||
if ( serverVersion.greaterThanOrEqual( v3_2_0 ) ) | ||
if ( serverVersion.greaterThanOrEqual( v4_0_0 )) |
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.
Maybe we should consider moving to compare protocol versions instead of server versions over time.
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.
Strictly speaking we cannot test bolt version as the procedures are not part of bolt protocol.
driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTableRegistryImpl.java
Outdated
Show resolved
Hide resolved
Fixed a flaky test due to misused timeout value
800d376
to
b6d1c2a
Compare
I lost one comment. I left a TODO to make |
GetRoutingTable
procedure now accepts an optional parameterdatabase
for fetching routing table for the specified database. To obtain routing table for default database, usedatabase = null
rather thandatabase = ''
.GetRoutingTable
procedure should be executed on system database as it is a DBMS procedure.Basics:
RoutingTableRegistry
keeps a concurrent map from database names toRoutingTableHandler
.RoutingTableHandler
holds aRoutingTable
(empty at creation) and handles removal of servers from routing table in case of connection errors.Discovery
provides discovery service. Given a routing table, it reads available routers and finds a connection from connection pool to perform discovery. If success, it updates the routing table and connection pool. If failed, it throws error.RoutingTableRegistry
,Discovery
andConnectionPool
are shared resources by all threads in drivers.RoutingTableHandler
andRoutingTable
is (mostly) used by a single thread.