Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delete ConfigBuilder.withRoutingFailureLimit and ConfigBuilder.withRoutingRetryDelay #1151

Merged
merged 1 commit into from
Feb 9, 2022
Merged

Delete ConfigBuilder.withRoutingFailureLimit and ConfigBuilder.withRoutingRetryDelay #1151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions driver/clirr-ignored-differences.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
88 changes: 1 addition & 87 deletions driver/src/main/java/org/neo4j/driver/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -233,7 +229,7 @@ SecuritySettings securitySettings()

RoutingSettings routingSettings()
{
return new RoutingSettings( routingFailureLimit, routingRetryDelayMillis, routingTablePurgeDelayMillis );
return new RoutingSettings( routingTablePurgeDelayMillis );
}

RetrySettings retrySettings()
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>
Expand Down
35 changes: 6 additions & 29 deletions driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 );
}

Expand All @@ -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(
Expand All @@ -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 );
}
} );
}
Expand Down
24 changes: 5 additions & 19 deletions driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 3 additions & 4 deletions driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 );
}

Expand Down Expand Up @@ -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 )
Expand Down
Loading