diff --git a/driver/clirr-ignored-differences.xml b/driver/clirr-ignored-differences.xml index c107db03ab..072d53608b 100644 --- a/driver/clirr-ignored-differences.xml +++ b/driver/clirr-ignored-differences.xml @@ -67,4 +67,16 @@ void reset() + + org/neo4j/driver/Config$ConfigBuilder + 7002 + org.neo4j.driver.Config$ConfigBuilder withRoutingFailureLimit(int) + + + + org/neo4j/driver/Config$ConfigBuilder + 7002 + org.neo4j.driver.Config$ConfigBuilder withRoutingRetryDelay(long, java.util.concurrent.TimeUnit) + + diff --git a/driver/src/main/java/org/neo4j/driver/Config.java b/driver/src/main/java/org/neo4j/driver/Config.java index 4a5398225e..20bb5d37ff 100644 --- a/driver/src/main/java/org/neo4j/driver/Config.java +++ b/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. - *

- * 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. - *

- * 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)}. - * Method will be removed in the next major release. - */ - @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. - *

- * 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. - *

- * 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. - *

- * 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)}. - * Method will be removed in the next major release. - */ - @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. *

diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java index 368711e036..692afa3063 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java +++ b/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 lookupClusterComposition( CompletableFuture 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 result, Bookmark bookmark, String impersonatedUser, - Throwable baseError ) + private void lookupClusterComposition( RoutingTable routingTable, ConnectionPool pool, CompletableFuture 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 ); } } ); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java index 605df0cc2c..376d5038ad 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java +++ b/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() diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java index 10d5dc71b6..041acd02fa 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java +++ b/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 ) diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java index cad1046464..f3a8131e0c 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java @@ -18,7 +18,6 @@ */ package org.neo4j.driver.internal.cluster; -import io.netty.util.concurrent.GlobalEventExecutor; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -432,57 +431,19 @@ void shouldUseKnownRoutersWhenInitialRouterFails() verify( table ).forget( D ); } - @Test - void shouldRetryConfiguredNumberOfTimesWithDelay() - { - int maxRoutingFailures = 3; - long retryTimeoutDelay = 15; - ClusterComposition expectedComposition = - new ClusterComposition( 42, asOrderedSet( A, C ), asOrderedSet( B, D ), asOrderedSet( A, E ), null ); - - Map responsesByAddress = new HashMap<>(); - responsesByAddress.put( A, new ServiceUnavailableException( "Hi!" ) ); - responsesByAddress.put( B, new ServiceUnavailableException( "Hi!" ) ); - responsesByAddress.put( E, expectedComposition ); - - ClusterCompositionProvider compositionProvider = compositionProviderMock( responsesByAddress ); - ServerAddressResolver resolver = mock( ServerAddressResolver.class ); - when( resolver.resolve( A ) ).thenReturn( asOrderedSet( A ) ) - .thenReturn( asOrderedSet( A ) ) - .thenReturn( asOrderedSet( E ) ); - - ImmediateSchedulingEventExecutor eventExecutor = new ImmediateSchedulingEventExecutor(); - RoutingSettings settings = new RoutingSettings( maxRoutingFailures, retryTimeoutDelay, 0 ); - Rediscovery rediscovery = - new RediscoveryImpl( A, settings, compositionProvider, eventExecutor, resolver, DEV_NULL_LOGGING, - DefaultDomainNameResolver.getInstance() ); - RoutingTable table = routingTableMock( A, B ); - - ClusterComposition actualComposition = await( rediscovery.lookupClusterComposition( table, pool, empty(), null ) ).getClusterComposition(); - - assertEquals( expectedComposition, actualComposition ); - verify( table, times( maxRoutingFailures ) ).forget( A ); - verify( table, times( maxRoutingFailures ) ).forget( B ); - assertEquals( asList( retryTimeoutDelay, retryTimeoutDelay * 2 ), eventExecutor.scheduleDelays() ); - } - @Test void shouldNotLogWhenSingleRetryAttemptFails() { - int maxRoutingFailures = 1; - long retryTimeoutDelay = 10; - Map responsesByAddress = singletonMap( A, new ServiceUnavailableException( "Hi!" ) ); ClusterCompositionProvider compositionProvider = compositionProviderMock( responsesByAddress ); ServerAddressResolver resolver = resolverMock( A, A ); ImmediateSchedulingEventExecutor eventExecutor = new ImmediateSchedulingEventExecutor(); - RoutingSettings settings = new RoutingSettings( maxRoutingFailures, retryTimeoutDelay, 0 ); Logging logging = mock( Logging.class ); Logger logger = mock( Logger.class ); when( logging.getLog( any( Class.class ) ) ).thenReturn( logger ); Rediscovery rediscovery = - new RediscoveryImpl( A, settings, compositionProvider, eventExecutor, resolver, logging, DefaultDomainNameResolver.getInstance() ); + new RediscoveryImpl( A, compositionProvider, resolver, logging, DefaultDomainNameResolver.getInstance() ); RoutingTable table = routingTableMock( A ); ServiceUnavailableException e = @@ -502,7 +463,7 @@ void shouldResolveToIP() throws UnknownHostException DomainNameResolver domainNameResolver = mock( DomainNameResolver.class ); InetAddress localhost = InetAddress.getLocalHost(); when( domainNameResolver.resolve( A.host() ) ).thenReturn( new InetAddress[]{localhost} ); - Rediscovery rediscovery = new RediscoveryImpl( A, null, null, null, resolver, DEV_NULL_LOGGING, domainNameResolver ); + Rediscovery rediscovery = new RediscoveryImpl( A, null, resolver, DEV_NULL_LOGGING, domainNameResolver ); List addresses = rediscovery.resolve(); @@ -521,8 +482,7 @@ private Rediscovery newRediscovery( BoltServerAddress initialRouter, ClusterComp private Rediscovery newRediscovery( BoltServerAddress initialRouter, ClusterCompositionProvider compositionProvider, ServerAddressResolver resolver, Logging logging ) { - RoutingSettings settings = new RoutingSettings( 1, 0, 0 ); - return new RediscoveryImpl( initialRouter, settings, compositionProvider, GlobalEventExecutor.INSTANCE, resolver, logging, + return new RediscoveryImpl( initialRouter, compositionProvider, resolver, logging, DefaultDomainNameResolver.getInstance() ); }