Small cleanup in Rediscovery

Extracted method that contacts specified router out of the loop and
better method naming.

Author: lutovich

driver/src/main/java/org/neo4j/driver/internal/cluster/LoadBalancer.java

@@ -127,23 +127,17 @@ synchronized void ensureRouting() throws ServiceUnavailableException, ProtocolEx
         if ( routingTable.isStale() )
         {
             log.info( "Routing information is stale. %s", routingTable );
-            try
-            {
-                // get a new routing table
-                ClusterComposition cluster = rediscovery.lookupRoutingTable( connections, routingTable );
-                Set<BoltServerAddress> removed = routingTable.update( cluster );
-                // purge connections to removed addresses
-                for ( BoltServerAddress address : removed )
-                {
-                    connections.purge( address );
-                }
 
-                log.info( "Refreshed routing information. %s", routingTable );
-            }
-            catch ( InterruptedException e )
+            // get a new routing table
+            ClusterComposition cluster = rediscovery.lookupClusterComposition( connections, routingTable );
+            Set<BoltServerAddress> removed = routingTable.update( cluster );
+            // purge connections to removed addresses
+            for ( BoltServerAddress address : removed )
             {
-                throw new ServiceUnavailableException( "Thread was interrupted while establishing connection.", e );
+                connections.purge( address );
             }
+
+            log.info( "Refreshed routing information. %s", routingTable );
         }
     }
 

driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java

@@ -23,8 +23,8 @@
 import org.neo4j.driver.internal.spi.ConnectionPool;
 import org.neo4j.driver.internal.util.Clock;
 import org.neo4j.driver.v1.Logger;
-import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
 import org.neo4j.driver.v1.exceptions.SecurityException;
+import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
 
 import static java.lang.String.format;
 
@@ -47,59 +47,94 @@ public Rediscovery( RoutingSettings settings, Clock clock, Logger logger, Cluste
 
     // Given the current routing table and connection pool, use the connection composition provider to fetch a new
     // cluster composition, which would be used to update the routing table and connection pool
-    public ClusterComposition lookupRoutingTable( ConnectionPool connections, RoutingTable routingTable )
-            throws InterruptedException
+    public ClusterComposition lookupClusterComposition( ConnectionPool connections, RoutingTable routingTable )
     {
         assertHasRouters( routingTable );
         int failures = 0;
 
         for ( long start = clock.millis(), delay = 0; ; delay = Math.max( settings.retryTimeoutDelay, delay * 2 ) )
         {
             long waitTime = start + delay - clock.millis();
-            if ( waitTime > 0 )
+            sleep( waitTime );
+            start = clock.millis();
+
+            ClusterComposition composition = lookupClusterCompositionOnKnownRouters( connections, routingTable );
+            if ( composition != null )
             {
-                clock.sleep( waitTime );
+                return composition;
+            }
+
+            if ( ++failures >= settings.maxRoutingFailures )
+            {
+                throw new ServiceUnavailableException( NO_ROUTERS_AVAILABLE );
+            }
+        }
+    }
+
+    private ClusterComposition lookupClusterCompositionOnKnownRouters( ConnectionPool connections,
+            RoutingTable routingTable )
+    {
+        int size = routingTable.routerSize();
+        for ( int i = 0; i < size; i++ )
+        {
+            BoltServerAddress address = routingTable.nextRouter();
+            if ( address == null )
+            {
+                throw new ServiceUnavailableException( NO_ROUTERS_AVAILABLE );
             }
-            start = clock.millis();
 
-            int size = routingTable.routerSize();
-            for ( int i = 0; i < size; i++ )
+            ClusterComposition composition = lookupClusterCompositionOnRouter( address, connections, routingTable );
+            if ( composition != null )
             {
-                BoltServerAddress address = routingTable.nextRouter();
-                if ( address == null )
-                {
-                    throw new ServiceUnavailableException( NO_ROUTERS_AVAILABLE );
-                }
+                return composition;
+            }
+        }
+        return null;
+    }
 
-                ClusterCompositionResponse response = null;
-                try ( Connection connection = connections.acquire( address ) )
-                {
-                    response = provider.getClusterComposition( connection );
-                }
-                catch( SecurityException e )
-                {
-                    throw e; // terminate the discovery immediately
-                }
-                catch ( Exception e )
-                {
-                    // the connection breaks
-                    logger.error( format( "Failed to connect to routing server '%s'.", address ), e );
-                    routingTable.removeRouter( address );
+    private ClusterComposition lookupClusterCompositionOnRouter( BoltServerAddress routerAddress,
+            ConnectionPool connections, RoutingTable routingTable )
+    {
+        ClusterCompositionResponse response;
+        try ( Connection connection = connections.acquire( routerAddress ) )
+        {
+            response = provider.getClusterComposition( connection );
+        }
+        catch ( SecurityException e )
+        {
+            // auth error happened, terminate the discovery procedure immediately
+            throw e;
+        }
+        catch ( Throwable t )
+        {
+            // connection turned out to be broken
+            logger.error( format( "Failed to connect to routing server '%s'.", routerAddress ), t );
+            routingTable.removeRouter( routerAddress );
+            return null;
+        }
 
-                    assertHasRouters( routingTable );
-                    continue;
-                }
+        ClusterComposition cluster = response.clusterComposition();
+        logger.info( "Got cluster composition %s", cluster );
+        if ( cluster.hasWriters() )
+        {
+            return cluster;
+        }
+        return null;
+    }
 
-                ClusterComposition cluster = response.clusterComposition();
-                logger.info( "Got cluster composition %s", cluster );
-                if ( cluster.hasWriters() )
-                {
-                    return cluster;
-                }
+    private void sleep( long millis )
+    {
+        if ( millis > 0 )
+        {
+            try
+            {
+                clock.sleep( millis );
             }
-            if ( ++failures >= settings.maxRoutingFailures )
+            catch ( InterruptedException e )
             {
-                throw new ServiceUnavailableException( NO_ROUTERS_AVAILABLE );
+                // restore the interrupted status
+                Thread.currentThread().interrupt();
+                throw new ServiceUnavailableException( "Thread was interrupted while performing discovery", e );
             }
         }
     }

driver/src/test/java/org/neo4j/driver/internal/cluster/LoadBalancerTest.java

@@ -78,7 +78,7 @@ public void ensureRoutingShouldUpdateRoutingTableAndPurgeConnectionPoolWhenStale
         // then
         assertNotNull( balancer );
         InOrder inOrder = inOrder( rediscovery, routingTable, conns );
-        inOrder.verify( rediscovery ).lookupRoutingTable( conns, routingTable );
+        inOrder.verify( rediscovery ).lookupClusterComposition( conns, routingTable );
         inOrder.verify( routingTable ).update( any( ClusterComposition.class ) );
         inOrder.verify( conns ).purge( new BoltServerAddress( "abc", 12 ) );
     }

driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java

@@ -93,7 +93,7 @@ public void shouldTryConfiguredMaxRoutingFailures() throws Exception
             // when
             try
             {
-                rediscovery.lookupRoutingTable( mock( ConnectionPool.class ), routingTable );
+                rediscovery.lookupClusterComposition( mock( ConnectionPool.class ), routingTable );
                 fail("Should fail as failed to discovery");
             }
             catch( ServiceUnavailableException e )
@@ -327,7 +327,7 @@ private static ClusterComposition rediscover( ConnectionPool connections, Routin
         Logger mockedLogger = mock( Logger.class );
 
         Rediscovery rediscovery = new Rediscovery( defaultRoutingSettings, mockedClock, mockedLogger, provider );
-        return rediscovery.lookupRoutingTable( connections, routingTable );
+        return rediscovery.lookupClusterComposition( connections, routingTable );
     }
 
     private static class TestRoutingTable extends ClusterRoutingTable