From aa9e9cc1bc812820ee3a53ded0cab2fe62240fc7 Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 12 Apr 2017 13:58:52 +0200 Subject: [PATCH] Treat one router as valid in routing table Previously driver considered routing table with single router to be stale and had to perform rediscovery before read/write transaction. Requirement to have more than 1 router is quite strict and can easily be violated by partially unavailable clusters. Additional rediscoveries in such cases add more load on the available core server. This commit makes driver tread routing table with single router as not stale, given that other non-staleness requirements are satisfied as well. --- .../internal/cluster/ClusterRoutingTable.java | 2 +- .../internal/RoutingDriverBoltKitTest.java | 46 ++++++++++++------- .../cluster/ClusterRoutingTableTest.java | 16 +++++++ .../test/resources/discover_one_router.script | 9 ++++ 4 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 driver/src/test/resources/discover_one_router.script diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java index 2bfafd4805..54a80d60d7 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/ClusterRoutingTable.java @@ -60,7 +60,7 @@ private ClusterRoutingTable( Clock clock ) public boolean isStaleFor( AccessMode mode ) { return expirationTimeout < clock.millis() || - routers.size() <= MIN_ROUTERS || + routers.size() < MIN_ROUTERS || mode == AccessMode.READ && readers.size() == 0 || mode == AccessMode.WRITE && writers.size() == 0; } diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java index 5370643800..d893fbeac3 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java @@ -963,22 +963,7 @@ public void shouldAcceptRoutingTableWithoutWritersAndThenRediscover() throws Exc // start another router which knows about writes, use same address as the initial router router2 = StubServer.start( "acquire_endpoints.script", 9010 ); - List names = session.readTransaction( new TransactionWork>() - { - @Override - public List execute( Transaction tx ) - { - List records = tx.run( "MATCH (n) RETURN n.name" ).list(); - List names = new ArrayList<>( records.size() ); - for ( Record record : records ) - { - names.add( record.get( 0 ).asString() ); - } - return names; - } - } ); - - assertEquals( asList( "Bob", "Alice", "Tina" ), names ); + assertEquals( asList( "Bob", "Alice", "Tina" ), readStrings( "MATCH (n) RETURN n.name", session ) ); StatementResult createResult = session.run( "CREATE (n {name:'Bob'})" ); assertFalse( createResult.hasNext() ); @@ -993,6 +978,35 @@ public List execute( Transaction tx ) } } + @Test + public void shouldTreatRoutingTableWithSingleRouterAsValid() throws Exception + { + StubServer router = StubServer.start( "discover_one_router.script", 9010 ); + StubServer reader1 = StubServer.start( "read_server.script", 9003 ); + StubServer reader2 = StubServer.start( "read_server.script", 9004 ); + + try ( Driver driver = GraphDatabase.driver( "bolt+routing://127.0.0.1:9010", config ); + Session session = driver.session( AccessMode.READ ) ) + { + // returned routing table contains only one router, this should be fine and we should be able to + // read multiple times without additional rediscovery + + StatementResult readResult1 = session.run( "MATCH (n) RETURN n.name" ); + assertEquals( "127.0.0.1:9003", readResult1.summary().server().address() ); + assertEquals( 3, readResult1.list().size() ); + + StatementResult readResult2 = session.run( "MATCH (n) RETURN n.name" ); + assertEquals( "127.0.0.1:9004", readResult2.summary().server().address() ); + assertEquals( 3, readResult2.list().size() ); + } + finally + { + assertEquals( 0, router.exitStatus() ); + assertEquals( 0, reader1.exitStatus() ); + assertEquals( 0, reader2.exitStatus() ); + } + } + private static Driver newDriverWithSleeplessClock( String uriString ) { DriverFactory driverFactory = new DriverFactoryWithClock( new SleeplessClock() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterRoutingTableTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterRoutingTableTest.java index fe4e338da6..0a12c079cf 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterRoutingTableTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/ClusterRoutingTableTest.java @@ -26,6 +26,7 @@ import org.neo4j.driver.internal.util.FakeClock; import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -181,4 +182,19 @@ public void shouldPreserveOrderingOfReaders() assertEquals( D, routingTable.readers().next() ); assertEquals( B, routingTable.readers().next() ); } + + @Test + public void shouldTreatOneRouterAsValid() + { + ClusterRoutingTable routingTable = new ClusterRoutingTable( new FakeClock() ); + + List routers = singletonList( A ); + List writers = asList( B, C ); + List readers = asList( D, E ); + + routingTable.update( createClusterComposition( routers, writers, readers ) ); + + assertFalse( routingTable.isStaleFor( READ ) ); + assertFalse( routingTable.isStaleFor( WRITE ) ); + } } diff --git a/driver/src/test/resources/discover_one_router.script b/driver/src/test/resources/discover_one_router.script new file mode 100644 index 0000000000..46e6505312 --- /dev/null +++ b/driver/src/test/resources/discover_one_router.script @@ -0,0 +1,9 @@ +!: AUTO INIT +!: AUTO RESET +!: AUTO PULL_ALL + +C: RUN "CALL dbms.cluster.routing.getServers" {} + PULL_ALL +S: SUCCESS {"fields": ["ttl", "servers"]} + RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9001","127.0.0.1:9002"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9003","127.0.0.1:9004"], "role": "READ"},{"addresses": ["127.0.0.1:9005"], "role": "ROUTE"}]] + SUCCESS {}