Delete ConfigBuilder.withRoutingFailureLimit and ConfigBuilder.withRoutingRetryDelay (#1151)

Author: injectives

driver/clirr-ignored-differences.xml

@@ -67,4 +67,16 @@
         <method>void reset()</method>
     </difference>
 
+    <difference>
+        <className>org/neo4j/driver/Config$ConfigBuilder</className>
+        <differenceType>7002</differenceType>
+        <method>org.neo4j.driver.Config$ConfigBuilder withRoutingFailureLimit(int)</method>
+    </difference>
+
+    <difference>
+        <className>org/neo4j/driver/Config$ConfigBuilder</className>
+        <differenceType>7002</differenceType>
+        <method>org.neo4j.driver.Config$ConfigBuilder withRoutingRetryDelay(long, java.util.concurrent.TimeUnit)</method>
+    </difference>
+
 </differences>

driver/src/main/java/org/neo4j/driver/Config.java

@@ -83,8 +83,6 @@ public class Config implements Serializable
 
     private final SecuritySettings securitySettings;
 
-    private final int routingFailureLimit;
-    private final long routingRetryDelayMillis;
     private final long fetchSize;
     private final long routingTablePurgeDelayMillis;
 
@@ -109,8 +107,6 @@ private Config( ConfigBuilder builder )
 
         this.securitySettings = builder.securitySettingsBuilder.build();
 
-        this.routingFailureLimit = builder.routingFailureLimit;
-        this.routingRetryDelayMillis = builder.routingRetryDelayMillis;
         this.connectionTimeoutMillis = builder.connectionTimeoutMillis;
         this.routingTablePurgeDelayMillis = builder.routingTablePurgeDelayMillis;
         this.retrySettings = builder.retrySettings;
@@ -233,7 +229,7 @@ SecuritySettings securitySettings()
 
     RoutingSettings routingSettings()
     {
-        return new RoutingSettings( routingFailureLimit, routingRetryDelayMillis, routingTablePurgeDelayMillis );
+        return new RoutingSettings( routingTablePurgeDelayMillis );
     }
 
     RetrySettings retrySettings()
@@ -280,8 +276,6 @@ public static class ConfigBuilder
         private long connectionAcquisitionTimeoutMillis = PoolSettings.DEFAULT_CONNECTION_ACQUISITION_TIMEOUT;
         private String userAgent = format( "neo4j-java/%s", driverVersion() );
         private final SecuritySettings.SecuritySettingsBuilder securitySettingsBuilder = new SecuritySettings.SecuritySettingsBuilder();
-        private int routingFailureLimit = RoutingSettings.DEFAULT.maxRoutingFailures();
-        private long routingRetryDelayMillis = RoutingSettings.DEFAULT.retryTimeoutDelay();
         private long routingTablePurgeDelayMillis = RoutingSettings.DEFAULT.routingTablePurgeDelayMs();
         private int connectionTimeoutMillis = (int) TimeUnit.SECONDS.toMillis( 30 );
         private RetrySettings retrySettings = RetrySettings.DEFAULT;
@@ -487,86 +481,6 @@ public ConfigBuilder withTrustStrategy( TrustStrategy trustStrategy )
             return this;
         }
 
-        /**
-         * Specify how many times the client should attempt to reconnect to the routing servers before declaring the
-         * cluster unavailable.
-         * <p>
-         * The routing servers are tried in order. If connecting any of them fails, they are all retried after
-         * {@linkplain #withRoutingRetryDelay a delay}. This process of retrying all servers is then repeated for the
-         * number of times specified here before considering the cluster unavailable.
-         * <p>
-         * The default value of this parameter is {@code 1}, which means that the the driver will not re-attempt to
-         * connect to the cluster when connecting has failed to each individual server in the list of routers. This
-         * default value is sensible under this assumption that if the attempt to connect fails for all servers, then
-         * the entire cluster is down, or the client is disconnected from the network, and retrying to connect will
-         * not bring it back up, in which case it is better to report the failure sooner.
-         *
-         * @param routingFailureLimit
-         *         the number of times to retry each server in the list of routing servers
-         * @return this builder
-         * @deprecated in 1.2 because driver memorizes seed URI used during construction and falls back to it at
-         * runtime when all other known router servers failed to respond. Driver is also able to perform DNS lookup
-         * for the seed URI during rediscovery. This means updates of cluster members will be picked up if they are
-         * reflected in a DNS record. This configuration allowed driver to retry rediscovery procedure and postpone
-         * failure. Currently there exists a better way of doing retries via
-         * {@link Session#readTransaction(TransactionWork)} and {@link Session#writeTransaction(TransactionWork)}.
-         * <b>Method will be removed in the next major release.</b>
-         */
-        @Deprecated
-        public ConfigBuilder withRoutingFailureLimit( int routingFailureLimit )
-        {
-            if ( routingFailureLimit < 1 )
-            {
-                throw new IllegalArgumentException(
-                        "The failure limit may not be smaller than 1, but was: " + routingFailureLimit );
-            }
-            this.routingFailureLimit = routingFailureLimit;
-            return this;
-        }
-
-        /**
-         * Specify how long to wait before retrying to connect to a routing server.
-         * <p>
-         * When connecting to all routing servers fail, connecting will be retried after the delay specified here.
-         * The delay is measured from when the first attempt to connect was made, so that the delay time specifies a
-         * retry interval.
-         * <p>
-         * For each {@linkplain #withRoutingFailureLimit retry attempt} the delay time will be doubled. The time
-         * specified here is the base time, i.e. the time to wait before the first retry. If that attempt (on all
-         * servers) also fails, the delay before the next retry will be double the time specified here, and the next
-         * attempt after that will be double that, et.c. So if, for example, the delay specified here is
-         * {@code 5 SECONDS}, then after attempting to connect to each server fails reconnecting will be attempted
-         * 5 seconds after the first connection attempt to the first server. If that attempt also fails to connect to
-         * all servers, the next attempt will start 10 seconds after the second attempt started.
-         * <p>
-         * The default value of this parameter is {@code 5 SECONDS}.
-         *
-         * @param delay
-         *         the amount of time between attempts to reconnect to the same server
-         * @param unit
-         *         the unit in which the duration is given
-         * @return this builder
-         * @deprecated in 1.2 because driver memorizes seed URI used during construction and falls back to it at
-         * runtime when all other known router servers failed to respond. Driver is also able to perform DNS lookup
-         * for the seed URI during rediscovery. This means updates of cluster members will be picked up if they are
-         * reflected in a DNS record. This configuration allowed driver to retry rediscovery procedure and postpone
-         * failure. Currently there exists a better way of doing retries via
-         * {@link Session#readTransaction(TransactionWork)} and {@link Session#writeTransaction(TransactionWork)}.
-         * <b>Method will be removed in the next major release.</b>
-         */
-        @Deprecated
-        public ConfigBuilder withRoutingRetryDelay( long delay, TimeUnit unit )
-        {
-            long routingRetryDelayMillis = unit.toMillis( delay );
-            if ( routingRetryDelayMillis < 0 )
-            {
-                throw new IllegalArgumentException( String.format(
-                        "The retry delay may not be smaller than 0, but was %d %s.", delay, unit ) );
-            }
-            this.routingRetryDelayMillis = routingRetryDelayMillis;
-            return this;
-        }
-
         /**
          * Specify how long to wait before purging stale routing tables.
          * <p>

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

@@ -18,8 +18,6 @@
  */
 package org.neo4j.driver.internal.cluster;
 
-import io.netty.util.concurrent.EventExecutorGroup;
-
 import java.net.UnknownHostException;
 import java.util.Collection;
 import java.util.HashSet;
@@ -29,7 +27,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.CompletionStage;
-import java.util.concurrent.TimeUnit;
 
 import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.Logger;
@@ -67,22 +64,18 @@ public class RediscoveryImpl implements Rediscovery
     private static final String INVALID_BOOKMARK_MIXTURE_CODE = "Neo.ClientError.Transaction.InvalidBookmarkMixture";
 
     private final BoltServerAddress initialRouter;
-    private final RoutingSettings settings;
     private final Logger log;
     private final ClusterCompositionProvider provider;
     private final ServerAddressResolver resolver;
-    private final EventExecutorGroup eventExecutorGroup;
     private final DomainNameResolver domainNameResolver;
 
-    public RediscoveryImpl( BoltServerAddress initialRouter, RoutingSettings settings, ClusterCompositionProvider provider,
-                            EventExecutorGroup eventExecutorGroup, ServerAddressResolver resolver, Logging logging, DomainNameResolver domainNameResolver )
+    public RediscoveryImpl( BoltServerAddress initialRouter, ClusterCompositionProvider provider, ServerAddressResolver resolver, Logging logging,
+                            DomainNameResolver domainNameResolver )
     {
         this.initialRouter = initialRouter;
-        this.settings = settings;
         this.log = logging.getLog( getClass() );
         this.provider = provider;
         this.resolver = resolver;
-        this.eventExecutorGroup = eventExecutorGroup;
         this.domainNameResolver = requireNonNull( domainNameResolver );
     }
 
@@ -101,13 +94,12 @@ public CompletionStage<ClusterCompositionLookupResult> lookupClusterComposition(
         CompletableFuture<ClusterCompositionLookupResult> result = new CompletableFuture<>();
         // if we failed discovery, we will chain all errors into this one.
         ServiceUnavailableException baseError = new ServiceUnavailableException( String.format( NO_ROUTERS_AVAILABLE, routingTable.database().description() ) );
-        lookupClusterComposition( routingTable, connectionPool, 0, 0, result, bookmark, impersonatedUser, baseError );
+        lookupClusterComposition( routingTable, connectionPool, result, bookmark, impersonatedUser, baseError );
         return result;
     }
 
-    private void lookupClusterComposition( RoutingTable routingTable, ConnectionPool pool, int failures, long previousDelay,
-                                           CompletableFuture<ClusterCompositionLookupResult> result, Bookmark bookmark, String impersonatedUser,
-                                           Throwable baseError )
+    private void lookupClusterComposition( RoutingTable routingTable, ConnectionPool pool, CompletableFuture<ClusterCompositionLookupResult> result,
+                                           Bookmark bookmark, String impersonatedUser, Throwable baseError )
     {
         lookup( routingTable, pool, bookmark, impersonatedUser, baseError )
                 .whenComplete(
@@ -124,22 +116,7 @@ else if ( compositionLookupResult != null )
                             }
                             else
                             {
-                                int newFailures = failures + 1;
-                                if ( newFailures >= settings.maxRoutingFailures() )
-                                {
-                                    // now we throw our saved error out
-                                    result.completeExceptionally( baseError );
-                                }
-                                else
-                                {
-                                    long nextDelay = Math.max( settings.retryTimeoutDelay(), previousDelay * 2 );
-                                    log.info( "Unable to fetch new routing table, will try again in " + nextDelay + "ms" );
-                                    eventExecutorGroup.next().schedule(
-                                            () -> lookupClusterComposition( routingTable, pool, newFailures, nextDelay, result, bookmark, impersonatedUser,
-                                                                            baseError ),
-                                            nextDelay, TimeUnit.MILLISECONDS
-                                    );
-                                }
+                                result.completeExceptionally( baseError );
                             }
                         } );
     }

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

@@ -23,39 +23,25 @@
 public class RoutingSettings
 {
     public static final long STALE_ROUTING_TABLE_PURGE_DELAY_MS = SECONDS.toMillis( 30 );
-    public static final RoutingSettings DEFAULT = new RoutingSettings( 1, SECONDS.toMillis( 5 ), STALE_ROUTING_TABLE_PURGE_DELAY_MS );
+    public static final RoutingSettings DEFAULT = new RoutingSettings( STALE_ROUTING_TABLE_PURGE_DELAY_MS );
 
-    private final int maxRoutingFailures;
-    private final long retryTimeoutDelay;
     private final RoutingContext routingContext;
     private final long routingTablePurgeDelayMs;
 
-    public RoutingSettings( int maxRoutingFailures, long retryTimeoutDelay, long routingTablePurgeDelayMs )
+    public RoutingSettings( long routingTablePurgeDelayMs )
     {
-        this( maxRoutingFailures, retryTimeoutDelay, routingTablePurgeDelayMs, RoutingContext.EMPTY );
+        this( routingTablePurgeDelayMs, RoutingContext.EMPTY );
     }
 
-    public RoutingSettings( int maxRoutingFailures, long retryTimeoutDelay, long routingTablePurgeDelayMs, RoutingContext routingContext )
+    public RoutingSettings( long routingTablePurgeDelayMs, RoutingContext routingContext )
     {
-        this.maxRoutingFailures = maxRoutingFailures;
-        this.retryTimeoutDelay = retryTimeoutDelay;
         this.routingContext = routingContext;
         this.routingTablePurgeDelayMs = routingTablePurgeDelayMs;
     }
 
     public RoutingSettings withRoutingContext( RoutingContext newRoutingContext )
     {
-        return new RoutingSettings( maxRoutingFailures, retryTimeoutDelay, routingTablePurgeDelayMs, newRoutingContext );
-    }
-
-    public int maxRoutingFailures()
-    {
-        return maxRoutingFailures;
-    }
-
-    public long retryTimeoutDelay()
-    {
-        return retryTimeoutDelay;
+        return new RoutingSettings( routingTablePurgeDelayMs, newRoutingContext );
     }
 
     public RoutingContext routingContext()

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

@@ -67,7 +67,6 @@ public class LoadBalancer implements ConnectionProvider
             "Failed to obtain connection towards %s server. Known routing table is: %s";
     private static final String CONNECTION_ACQUISITION_ATTEMPT_FAILURE_MESSAGE =
             "Failed to obtain a connection towards address %s, will try other addresses if available. Complete failure is reported separately from this entry.";
-    private static final BoltServerAddress[] BOLT_SERVER_ADDRESSES_EMPTY_ARRAY = new BoltServerAddress[0];
     private final ConnectionPool connectionPool;
     private final RoutingTableRegistry routingTables;
     private final LoadBalancingStrategy loadBalancingStrategy;
@@ -79,7 +78,7 @@ public LoadBalancer( BoltServerAddress initialRouter, RoutingSettings settings,
                          EventExecutorGroup eventExecutorGroup, Clock clock, Logging logging,
                          LoadBalancingStrategy loadBalancingStrategy, ServerAddressResolver resolver, DomainNameResolver domainNameResolver )
     {
-        this( connectionPool, createRediscovery( eventExecutorGroup, initialRouter, resolver, settings, clock, logging, requireNonNull( domainNameResolver ) ),
+        this( connectionPool, createRediscovery( initialRouter, resolver, settings, clock, logging, requireNonNull( domainNameResolver ) ),
               settings, loadBalancingStrategy, eventExecutorGroup, clock, logging );
     }
 
@@ -272,11 +271,11 @@ private static RoutingTableRegistry createRoutingTables( ConnectionPool connecti
         return new RoutingTableRegistryImpl( connectionPool, rediscovery, clock, logging, settings.routingTablePurgeDelayMs() );
     }
 
-    private static Rediscovery createRediscovery( EventExecutorGroup eventExecutorGroup, BoltServerAddress initialRouter, ServerAddressResolver resolver,
+    private static Rediscovery createRediscovery( BoltServerAddress initialRouter, ServerAddressResolver resolver,
                                                   RoutingSettings settings, Clock clock, Logging logging, DomainNameResolver domainNameResolver )
     {
         ClusterCompositionProvider clusterCompositionProvider = new RoutingProcedureClusterCompositionProvider( clock, settings.routingContext() );
-        return new RediscoveryImpl( initialRouter, settings, clusterCompositionProvider, eventExecutorGroup, resolver, logging, domainNameResolver );
+        return new RediscoveryImpl( initialRouter, clusterCompositionProvider, resolver, logging, domainNameResolver );
     }
 
     private static RuntimeException unknownMode( AccessMode mode )