Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 17 additions & 10 deletions driver/src/main/java/org/neo4j/driver/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -554,27 +554,34 @@ public ConfigBuilder withMaxConnectionPoolSize(int value) {
}

/**
* Configure maximum amount of time connection acquisition will attempt to acquire a connection from the
* connection pool. This timeout only kicks in when all existing connections are being used and no new
* connections can be created because maximum connection pool size has been reached.
* Sets the maximum amount of time the driver will wait to acquire a connection suitable for a given purpose.
* <p>
* Exception is raised when connection can't be acquired within configured time.
* In some situations the driver may need to do multiple actions and potentially acquire multiple connections to
* get a suitable one. For instance, when client-side routing is used, home database resolution is required,
* liveness checks are needed, etc.
* <p>
* Default value is 60 seconds. Negative values are allowed and result in unlimited acquisition timeout. Value
* of {@code 0} is allowed and results in no timeout and immediate failure when connection is unavailable.
* An exception is raised when connection can't be acquired within configured time.
* <p>
* Timeout value should be greater or equal to zero and represent a valid {@code long} value when converted to
* {@link TimeUnit#MILLISECONDS milliseconds}.
* <p>
* Default value is 60 seconds. {@literal 0} value disables timeout.
* <p>
* This timeout should be bigger than {@link ConfigBuilder#withConnectionTimeout(long, TimeUnit)}.
*
* @param value the acquisition timeout
* @param unit the unit in which the duration is given
* @return this builder
* @see #withMaxConnectionPoolSize(int)
* @see #withConnectionTimeout(long, TimeUnit)
*/
public ConfigBuilder withConnectionAcquisitionTimeout(long value, TimeUnit unit) {
var valueInMillis = unit.toMillis(value);
if (value >= 0) {
this.connectionAcquisitionTimeoutMillis = valueInMillis;
} else {
this.connectionAcquisitionTimeoutMillis = -1;
if (valueInMillis < 0) {
throw new IllegalArgumentException(format(
"The connection acquisition timeout may not be smaller than 0, but was %d %s.", value, unit));
}
this.connectionAcquisitionTimeoutMillis = valueInMillis;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ private RoutedBoltConnectionSource createRoutedBoltConnectionProvider(
clock,
loggingProvider,
uri,
config.connectionAcquisitionTimeoutMillis(),
List.of(AuthTokenManagerExecutionException.class),
observationProvider);
}
Expand Down Expand Up @@ -381,7 +382,8 @@ private BoltConnectionSourceFactory createPooledBoltConnectionSource(
boltAgent,
userAgent,
connectTimeoutMillis,
notificationConfig);
notificationConfig,
PooledBoltConnectionSource.ZeroTimeoutPolicy.DEFAULT);
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public CompletionStage<BoltConnection> connect(
BoltAgent boltAgent,
String userAgent,
int connectTimeoutMillis,
long initialisationTimeoutMillis,
SecurityPlan securityPlan,
AuthToken authToken,
BoltProtocolVersion minVersion,
Expand All @@ -56,6 +57,7 @@ public CompletionStage<BoltConnection> connect(
boltAgent,
userAgent,
connectTimeoutMillis,
initialisationTimeoutMillis,
securityPlan,
authToken,
minVersion,
Expand Down
9 changes: 4 additions & 5 deletions driver/src/test/java/org/neo4j/driver/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,11 @@ void shouldAllowPositiveConnectionAcquisitionTimeout() {
}

@Test
void shouldAllowNegativeConnectionAcquisitionTimeout() {
var config = Config.builder()
.withConnectionAcquisitionTimeout(-42, TimeUnit.HOURS)
.build();
void shouldRejectNegativeConnectionAcquisitionTimeout() {
var builder = Config.builder();

assertEquals(-1, config.connectionAcquisitionTimeoutMillis());
assertThrows(
IllegalArgumentException.class, () -> builder.withConnectionAcquisitionTimeout(-42, TimeUnit.HOURS));
}

@Test
Expand Down
46 changes: 24 additions & 22 deletions driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING;

import java.io.IOException;
import java.net.ServerSocket;
import java.net.URI;
import java.util.concurrent.TimeoutException;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.neo4j.driver.exceptions.Neo4jException;
import org.neo4j.driver.exceptions.ServiceUnavailableException;
import org.neo4j.driver.internal.security.StaticAuthTokenManager;
import org.neo4j.driver.testutil.TestUtil;
Expand All @@ -56,9 +58,12 @@ void shouldRespondToInterruptsWhenConnectingToUnresponsiveServer() throws Except
@SuppressWarnings("resource")
final var driver = GraphDatabase.driver(
"bolt://localhost:" + serverSocket.getLocalPort(),
Config.builder().withConnectionTimeout(1, SECONDS).build());
Config.builder()
.withConnectionTimeout(1, SECONDS)
.withConnectionAcquisitionTimeout(1, SECONDS)
.build());
try {
assertThrows(ServiceUnavailableException.class, driver::verifyConnectivity);
assertThrows(Neo4jException.class, driver::verifyConnectivity);
} finally {
// clear interrupted flag
Thread.interrupted();
Expand Down Expand Up @@ -171,27 +176,24 @@ private static void testFailureWhenServerDoesNotRespond(boolean encrypted) throw
try (var server = new ServerSocket(0)) // server that accepts connections but does not reply
{
var connectionTimeoutMillis = 1_000;
var config = createConfig(encrypted, connectionTimeoutMillis);
var configBuilder = Config.builder()
.withConnectionTimeout(connectionTimeoutMillis, MILLISECONDS)
.withConnectionAcquisitionTimeout(connectionTimeoutMillis, MILLISECONDS);
if (encrypted) {
configBuilder.withEncryption();
} else {
configBuilder.withoutEncryption();
}
@SuppressWarnings("resource")
final var driver = GraphDatabase.driver(URI.create("bolt://localhost:" + server.getLocalPort()), config);

var e = assertThrows(ServiceUnavailableException.class, driver::verifyConnectivity);
assertEquals(e.getMessage(), "Unable to establish connection in " + connectionTimeoutMillis + "ms");
}
}

private static Config createConfig(boolean encrypted, int timeoutMillis) {
@SuppressWarnings("deprecation")
var configBuilder = Config.builder()
.withConnectionTimeout(timeoutMillis, MILLISECONDS)
.withLogging(DEV_NULL_LOGGING);
final var driver = GraphDatabase.driver(
URI.create("bolt://localhost:" + server.getLocalPort()), configBuilder.build());

if (encrypted) {
configBuilder.withEncryption();
} else {
configBuilder.withoutEncryption();
var e = assertThrows(Neo4jException.class, driver::verifyConnectivity);
if (e instanceof ServiceUnavailableException) {
assertEquals("Unable to initialise connection in " + connectionTimeoutMillis + "ms", e.getMessage());
} else {
assertInstanceOf(TimeoutException.class, e.getCause());
}
}

return configBuilder.build();
}
}
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<maven.deploy.skip>true</maven.deploy.skip>

<!-- Versions -->
<neo4j-bolt-connection-bom.version>7.0.0</neo4j-bolt-connection-bom.version>
<neo4j-bolt-connection-bom.version>7.0-SNAPSHOT</neo4j-bolt-connection-bom.version>
<reactive-streams.version>1.0.4</reactive-streams.version>
<!-- Please note that when updating this dependency -->
<!-- (i.e. due to a security vulnerability or bug) that the -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,7 @@ public class StartTest implements TestkitRequest {
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.test_partial_summary_contains_updates$", "Does not contain updates because value is zero");
COMMON_SKIP_PATTERN_TO_REASON.put("^.*\\.test_supports_multi_db$", "Database is None");
var skipMessage = "Driver handles connection acquisition timeout differently";
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestConnectionAcquisitionTimeoutMs\\.test_should_encompass_the_handshake_time.*$", skipMessage);
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestConnectionAcquisitionTimeoutMs\\.test_router_handshake_has_own_timeout_too_slow$",
skipMessage);
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestConnectionAcquisitionTimeoutMs\\.test_should_fail_when_acquisition_timeout_is_reached_first.*$",
skipMessage);
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestConnectionAcquisitionTimeoutMs\\.test_should_encompass_the_version_handshake_(in_time|time_out)$",
skipMessage);
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestHomeDbMixedCluster\\.test_connection_acquisition_timeout_during_fallback$", skipMessage);
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestConnectionAcquisitionTimeoutMs\\.test_does_encompass_router_route_response$", skipMessage);
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestConnectionAcquisitionTimeoutMs\\.test_router_handshake_shares_acquisition_timeout$",
skipMessage);
skipMessage = "This test needs updating to implement expected behaviour";
var skipMessage = "This test needs updating to implement expected behaviour";
COMMON_SKIP_PATTERN_TO_REASON.put(
"^.*\\.TestAuthenticationSchemes[^.]+\\.test_custom_scheme_empty$", skipMessage);
skipMessage = "Driver does not implement optimization for qid in explicit transaction";
Expand Down