Make AddressSet retain resolved addresses

Routing table may override resolved router address in routing table. This leads to routing connection pool closures. This update aims to optimise this.
neo4j · Oct 14, 2021 · 46ea128 · 46ea128
1 parent 9034eb9
commit 46ea128
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 3 deletions.
diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/AddressSet.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/AddressSet.java
@@ -40,13 +40,23 @@ public int size()
         return addresses.length;
     }
 
+    /**
+     * Updates addresses using the provided set.
+     * <p>
+     * It aims to retain existing addresses by checking if they are present in the new set. To benefit from this, the provided set MUST contain specifically
+     * {@link BoltServerAddress} instances with equal host and connection host values.
+     *
+     * @param newAddresses the new address set.
+     */
     public synchronized void retainAllAndAdd( Set<BoltServerAddress> newAddresses )
     {
         BoltServerAddress[] addressesArr = new BoltServerAddress[newAddresses.size()];
         int insertionIdx = 0;
         for ( BoltServerAddress address : addresses )
         {
-            if ( newAddresses.remove( address ) )
+            BoltServerAddress lookupAddress =
+                    BoltServerAddress.class.equals( address.getClass() ) ? address : new BoltServerAddress( address.host(), address.port() );
+            if ( newAddresses.remove( lookupAddress ) )
             {
                 addressesArr[insertionIdx] = address;
                 insertionIdx++;

diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTableHandlerImpl.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingTableHandlerImpl.java
@@ -136,6 +136,7 @@ private synchronized void freshClusterCompositionFetched( ClusterCompositionLook
     {
         try
         {
+            log.debug( "Fetched cluster composition for database '%s'. %s", databaseName.description(), compositionLookupResult.getClusterComposition() );
             routingTable.update( compositionLookupResult.getClusterComposition() );
             routingTableRegistry.removeAged();
 
@@ -166,7 +167,8 @@ private synchronized void freshClusterCompositionFetched( ClusterCompositionLook
 
     private synchronized void clusterCompositionLookupFailed( Throwable error )
     {
-        log.error( String.format( "Failed to update routing table for database '%s'. Current routing table: %s.", databaseName.description(), routingTable ), error );
+        log.error( String.format( "Failed to update routing table for database '%s'. Current routing table: %s.", databaseName.description(), routingTable ),
+                   error );
         routingTableRegistry.remove( databaseName );
         CompletableFuture<RoutingTable> routingTableFuture = refreshRoutingTableFuture;
         refreshRoutingTableFuture = null;

diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/AddressSetTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/AddressSetTest.java
@@ -20,13 +20,18 @@
 
 import org.junit.jupiter.api.Test;
 
+import java.net.InetAddress;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
 import org.neo4j.driver.internal.BoltServerAddress;
+import org.neo4j.driver.internal.ResolvedBoltServerAddress;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
 
 class AddressSetTest
 {
@@ -142,6 +147,32 @@ void shouldHaveCorrectSize()
         assertEquals( 2, addressSet.size() );
     }
 
+    @Test
+    void shouldRetainExistingAddresses()
+    {
+        AddressSet addressSet = new AddressSet();
+        BoltServerAddress address0 = new BoltServerAddress( "node0", 7687 );
+        BoltServerAddress address1 = new ResolvedBoltServerAddress( "node1", 7687, new InetAddress[]{InetAddress.getLoopbackAddress()} );
+        BoltServerAddress address2 = new BoltServerAddress( "node2", 7687 );
+        BoltServerAddress address3 = new BoltServerAddress( "node3", 7687 );
+        BoltServerAddress address4 = new BoltServerAddress( "node4", 7687 );
+        addressSet.retainAllAndAdd( new HashSet<>( Arrays.asList( address0, address1, address2, address3, address4 ) ) );
+
+        BoltServerAddress sameAddress0 = new BoltServerAddress( "node0", 7687 );
+        BoltServerAddress sameAddress1 = new BoltServerAddress( "node1", 7687 );
+        BoltServerAddress differentAddress2 = new BoltServerAddress( "different-node2", 7687 );
+        BoltServerAddress sameAddress3 = new BoltServerAddress( "node3", 7687 );
+        BoltServerAddress sameAddress4 = new BoltServerAddress( "node4", 7687 );
+        addressSet.retainAllAndAdd( new HashSet<>( Arrays.asList( sameAddress0, sameAddress1, differentAddress2, sameAddress3, sameAddress4 ) ) );
+
+        assertEquals( 5, addressSet.size() );
+        assertSame( addressSet.toArray()[0], address0 );
+        assertSame( addressSet.toArray()[1], address1 );
+        assertSame( addressSet.toArray()[2], address3 );
+        assertSame( addressSet.toArray()[3], address4 );
+        assertSame( addressSet.toArray()[4], differentAddress2 );
+    }
+
     private static Set<BoltServerAddress> addresses( String... strings )
     {
         Set<BoltServerAddress> set = new LinkedHashSet<>();

diff --git a/...it-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/GetRoutingTable.java b/...it-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/GetRoutingTable.java
@@ -61,8 +61,12 @@ public TestkitResponse process( TestkitState testkitState )
                         String.format( "There is no routing table handler for the '%s' database.", databaseName.databaseName().orElse( "null" ) ) ) );
 
         org.neo4j.driver.internal.cluster.RoutingTable routingTable = routingTableHandler.routingTable();
+
+        Function<BoltServerAddress,String> addressToString = address -> String.format( "%s:%d", address.host(), address.port() );
+
         Function<AddressSet,List<String>> addressesToStrings = ( addresses ) -> Arrays.stream( addresses.toArray() )
-                                                                                      .map( BoltServerAddress::toString ).collect( Collectors.toList() );
+                                                                                      .map( addressToString )
+                                                                                      .collect( Collectors.toList() );
 
         return RoutingTable
                 .builder()