Skip to content

Changing default conneciton timeout to be 30s #629

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.

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

Merged
merged 3 commits into from
Oct 15, 2019
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public BeginMessage( Bookmarks bookmarks, TransactionConfig config, AccessMode m

public BeginMessage( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
{
super( bookmarks, txTimeout, txMetadata, mode );
super( buildMetadata( bookmarks, txTimeout, txMetadata, mode ) );
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,41 @@

import org.neo4j.driver.internal.Bookmarks;
import org.neo4j.driver.v1.AccessMode;
import org.neo4j.driver.v1.Statement;
import org.neo4j.driver.v1.TransactionConfig;
import org.neo4j.driver.v1.Value;

import static java.util.Collections.emptyMap;
import static org.neo4j.driver.v1.Values.ofValue;

public class RunWithMetadataMessage extends TransactionStartingMessage
{
public final static byte SIGNATURE = 0x10;

private final String statement;
private final Map<String,Value> parameters;

public RunWithMetadataMessage( String statement, Map<String,Value> parameters, Bookmarks bookmarks, TransactionConfig config, AccessMode mode )
public static RunWithMetadataMessage autoCommitTxRunMessage( Statement statement, TransactionConfig config, AccessMode mode,
Bookmarks bookmark )
{
return autoCommitTxRunMessage( statement.text(), statement.parameters().asMap( ofValue() ), config.timeout(), config.metadata(), mode, bookmark );
}

public static RunWithMetadataMessage autoCommitTxRunMessage( String statement, Map<String,Value> parameters, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode,
Bookmarks bookmark )
{
Map<String,Value> metadata = buildMetadata( bookmark, txTimeout, txMetadata, mode );
return new RunWithMetadataMessage( statement, parameters, metadata );
}

public static RunWithMetadataMessage explicitTxRunMessage( Statement statement )
{
this( statement, parameters, bookmarks, config.timeout(), config.metadata(), mode );
return new RunWithMetadataMessage( statement.text(), statement.parameters().asMap( ofValue() ), emptyMap() );
}

public RunWithMetadataMessage( String statement, Map<String,Value> parameters, Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata,
AccessMode mode )
public RunWithMetadataMessage( String statement, Map<String,Value> parameters, Map<String,Value> metadata )
{
super( bookmarks, txTimeout, txMetadata, mode );
super( metadata );
this.statement = statement;
this.parameters = parameters;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.neo4j.driver.v1.Value;

import static java.util.Collections.emptyMap;
import static java.util.Objects.requireNonNull;
import static org.neo4j.driver.v1.Values.value;

abstract class TransactionStartingMessage implements Message
Expand All @@ -40,17 +41,17 @@ abstract class TransactionStartingMessage implements Message

final Map<String,Value> metadata;

TransactionStartingMessage( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
TransactionStartingMessage( Map<String,Value> metadata )
{
this.metadata = buildMetadata( bookmarks, txTimeout, txMetadata, mode );
this.metadata = requireNonNull( metadata );
}

public final Map<String,Value> metadata()
{
return metadata;
}

private static Map<String,Value> buildMetadata( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
static Map<String,Value> buildMetadata( Bookmarks bookmarks, Duration txTimeout, Map<String,Value> txMetadata, AccessMode mode )
{
boolean bookmarksPresent = bookmarks != null && !bookmarks.isEmpty();
boolean txTimeoutPresent = txTimeout != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.neo4j.driver.internal.messaging.request.BeginMessage;
import org.neo4j.driver.internal.messaging.request.GoodbyeMessage;
import org.neo4j.driver.internal.messaging.request.HelloMessage;
import org.neo4j.driver.internal.messaging.request.RunWithMetadataMessage;
import org.neo4j.driver.internal.spi.Connection;
import org.neo4j.driver.internal.util.Futures;
import org.neo4j.driver.internal.util.MetadataExtractor;
Expand All @@ -57,6 +56,8 @@
import static org.neo4j.driver.internal.messaging.request.CommitMessage.COMMIT;
import static org.neo4j.driver.internal.messaging.request.PullAllMessage.PULL_ALL;
import static org.neo4j.driver.internal.messaging.request.RollbackMessage.ROLLBACK;
import static org.neo4j.driver.internal.messaging.request.RunWithMetadataMessage.autoCommitTxRunMessage;
import static org.neo4j.driver.internal.messaging.request.RunWithMetadataMessage.explicitTxRunMessage;
import static org.neo4j.driver.v1.Values.ofValue;

public class BoltProtocolV3 implements BoltProtocol
Expand Down Expand Up @@ -131,24 +132,25 @@ public CompletionStage<Void> rollbackTransaction( Connection connection )
public CompletionStage<InternalStatementResultCursor> runInAutoCommitTransaction( Connection connection, Statement statement,
BookmarksHolder bookmarksHolder, TransactionConfig config, boolean waitForRunResponse )
{
return runStatement( connection, statement, bookmarksHolder, null, config, waitForRunResponse );
Message runMessage = autoCommitTxRunMessage( statement, config, connection.mode(), bookmarksHolder.getBookmarks() );
return runStatement( connection, statement, bookmarksHolder, null, waitForRunResponse, runMessage );
}

@Override
public CompletionStage<InternalStatementResultCursor> runInExplicitTransaction( Connection connection, Statement statement, ExplicitTransaction tx,
boolean waitForRunResponse )
{
return runStatement( connection, statement, BookmarksHolder.NO_OP, tx, TransactionConfig.empty(), waitForRunResponse );
Message runMessage = explicitTxRunMessage( statement );
return runStatement( connection, statement, BookmarksHolder.NO_OP, tx, waitForRunResponse, runMessage );
}

private static CompletionStage<InternalStatementResultCursor> runStatement( Connection connection, Statement statement,
BookmarksHolder bookmarksHolder, ExplicitTransaction tx, TransactionConfig config, boolean waitForRunResponse )
private static CompletionStage<InternalStatementResultCursor> runStatement( Connection connection, Statement statement, BookmarksHolder bookmarksHolder,
ExplicitTransaction tx, boolean waitForRunResponse, Message runMessage )
{
String query = statement.text();
Map<String,Value> params = statement.parameters().asMap( ofValue() );

CompletableFuture<Void> runCompletedFuture = new CompletableFuture<>();
Message runMessage = new RunWithMetadataMessage( query, params, bookmarksHolder.getBookmarks(), config, connection.mode() );
RunResponseHandler runHandler = new RunResponseHandler( runCompletedFuture, METADATA_EXTRACTOR );
PullAllResponseHandler pullAllHandler = newPullAllHandler( statement, runHandler, connection, bookmarksHolder, tx );

Expand Down
4 changes: 2 additions & 2 deletions driver/src/main/java/org/neo4j/driver/v1/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public static class ConfigBuilder
private LoadBalancingStrategy loadBalancingStrategy = LoadBalancingStrategy.LEAST_CONNECTED;
private int routingFailureLimit = RoutingSettings.DEFAULT.maxRoutingFailures();
private long routingRetryDelayMillis = RoutingSettings.DEFAULT.retryTimeoutDelay();
private int connectionTimeoutMillis = (int) TimeUnit.SECONDS.toMillis( 5 );
private int connectionTimeoutMillis = (int) TimeUnit.SECONDS.toMillis( 30 );
private RetrySettings retrySettings = RetrySettings.DEFAULT;
private ServerAddressResolver resolver;

Expand Down Expand Up @@ -687,7 +687,7 @@ public ConfigBuilder withRoutingRetryDelay( long delay, TimeUnit unit )
* Timeout value should be greater or equal to zero and represent a valid {@code int} value when converted to
* {@link TimeUnit#MILLISECONDS milliseconds}.
* <p>
* The default value of this parameter is {@code 5 SECONDS}.
* The default value of this parameter is {@code 30 SECONDS}.
*
* @param value the timeout duration
* @param unit the unit in which duration is given
Expand Down
3 changes: 2 additions & 1 deletion driver/src/main/java/org/neo4j/driver/v1/Driver.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
* The <a href="https://tools.ietf.org/html/rfc3986">URI</a> passed to
* this method determines the type of Driver created.
* <br>
* <table border="1" cellpadding="4" style="border-collapse: collapse" summary="Available schemes and drivers">
* <table border="1" style="border-collapse: collapse">
* <caption>Available schemes and drivers</caption>
* <thead>
* <tr><th>URI Scheme</th><th>Driver</th></tr>
* </thead>
Expand Down
3 changes: 2 additions & 1 deletion driver/src/main/java/org/neo4j/driver/v1/Logging.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
* <p>
* Driver logging API defines the following log levels: ERROR, INFO, WARN, DEBUG and TRACE. They are similar to levels defined by SLF4J but different from
* log levels defined for {@link java.util.logging}. The following mapping takes place:
* <table border="1" cellpadding="4" summary="Driver and JUL log levels">
* <table border="1" style="border-collapse: collapse">
* <caption>Driver and JUL log levels</caption>
* <tr>
* <th>Driver</th>
* <th>java.util.logging</th>
Expand Down
8 changes: 4 additions & 4 deletions driver/src/main/java/org/neo4j/driver/v1/Value.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue
/**
* If this value represents a list or map, test if the collection is empty.
*
* @return <tt>true</tt> if size() is 0, otherwise <tt>false</tt>
* @return {@code true} if size() is 0, otherwise {@code false}
*/
boolean isEmpty();

Expand Down Expand Up @@ -154,17 +154,17 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue
boolean hasType( Type type );

/**
* @return <tt>true</tt> if the value is a Boolean value and has the value True.
* @return {@code true} if the value is a Boolean value and has the value True.
*/
boolean isTrue();

/**
* @return <tt>true</tt> if the value is a Boolean value and has the value False.
* @return {@code true} if the value is a Boolean value and has the value False.
*/
boolean isFalse();

/**
* @return <tt>true</tt> if the value is a Null, otherwise <tt>false</tt>
* @return {@code true} if the value is a Null, otherwise {@code false}
*/
boolean isNull();

Expand Down
2 changes: 1 addition & 1 deletion driver/src/main/java/org/neo4j/driver/v1/Values.java
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ public static Function<Value,List<Object>> ofList()
}

/**
* Converts values to {@link List} of <tt>T</tt>.
* Converts values to {@link List} of {@code T}.
*
* @param innerMap converter for the values inside the list
* @param <T> the type of values inside the list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public interface MapAccessor
* Check if the list of keys contains the given key
*
* @param key the key
* @return <tt>true</tt> if this map keys contains the given key otherwise <tt>false</tt>
* @return {@code true} if this map keys contains the given key otherwise {@code false}
*/
boolean containsKey( String key );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.net.URI;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;

import org.neo4j.driver.internal.cluster.RoutingSettings;
import org.neo4j.driver.internal.retry.RetrySettings;
Expand Down Expand Up @@ -150,7 +151,7 @@ void shouldSendReadAccessModeInStatementMetadata() throws Exception
StubServer server = StubServer.start( "hello_run_exit_read.script", 9001 );

try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
Session session = driver.session( AccessMode.READ ) )
Session session = driver.session( AccessMode.READ ) )
{
List<String> names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() );
assertEquals( asList( "Foo", "Bar" ), names );
Expand All @@ -167,7 +168,7 @@ void shouldNotSendWriteAccessModeInStatementMetadata() throws Exception
StubServer server = StubServer.start( "hello_run_exit.script", 9001 );

try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
Session session = driver.session( AccessMode.WRITE ) )
Session session = driver.session( AccessMode.WRITE ) )
{
List<String> names = session.run( "MATCH (n) RETURN n.name" ).list( record -> record.get( 0 ).asString() );
assertEquals( asList( "Foo", "Bar" ), names );
Expand Down Expand Up @@ -209,36 +210,62 @@ void shouldCloseChannelWhenResetFails() throws Exception
}

@Test
void shouldPropagateTransactionCommitErrorWhenSessionClosed() throws Exception
void shouldPropagateTransactionRollbackErrorWhenSessionClosed() throws Exception
{
testTransactionCloseErrorPropagationWhenSessionClosed( "commit_error.script", true, "Unable to commit" );
StubServer server = StubServer.start( "rollback_error.script", 9001 );
try
{
try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ) )
{
Session session = driver.session();

Transaction tx = session.beginTransaction();
StatementResult result = tx.run( "CREATE (n {name:'Alice'}) RETURN n.name AS name" );
assertEquals( "Alice", result.single().get( "name" ).asString() );

TransientException e = assertThrows( TransientException.class, session::close );
assertEquals( "Neo.TransientError.General.DatabaseUnavailable", e.code() );
assertEquals( "Unable to rollback", e.getMessage() );
}
}
finally
{
assertEquals( 0, server.exitStatus() );
}
}

@Test
void shouldPropagateTransactionRollbackErrorWhenSessionClosed() throws Exception
void shouldThrowCommitErrorWhenTransactionCommit() throws Exception
{
testTransactionCloseErrorPropagationWhenSessionClosed( "rollback_error.script", false, "Unable to rollback" );
testTxCloseErrorPropagation( "commit_error.script", tx -> {
tx.success();
tx.close();
}, "Unable to commit" );
}

@Test
void shouldThrowCommitErrorWhenTransactionClosed() throws Exception
void shouldThrowRollbackErrorWhenTransactionRollback() throws Exception
{
testTxCloseErrorPropagation( "commit_error.script", true, "Unable to commit" );
testTxCloseErrorPropagation( "rollback_error.script", tx -> {
tx.failure();
tx.close();
}, "Unable to rollback" );
}

@Test
void shouldThrowRollbackErrorWhenTransactionClosed() throws Exception
void shouldThrowRollbackErrorWhenTransactionClose() throws Exception
{
testTxCloseErrorPropagation( "rollback_error.script", false, "Unable to rollback" );
testTxCloseErrorPropagation( "rollback_error.script", Transaction::close, "Unable to rollback" );
}


@Test
void shouldThrowCorrectErrorOnRunFailure() throws Throwable
{
StubServer server = StubServer.start( "database_shutdown.script", 9001 );

try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
Session session = driver.session( "neo4j:bookmark:v1:tx0" );
Session session = driver.session( "neo4j:bookmark:v1:tx0" );
// has to enforce to flush BEGIN to have tx started.
Transaction transaction = session.beginTransaction() )
{
Expand All @@ -260,7 +287,7 @@ void shouldThrowCorrectErrorOnCommitFailure() throws Throwable
StubServer server = StubServer.start( "database_shutdown_at_commit.script", 9001 );

try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG );
Session session = driver.session() )
Session session = driver.session() )
{
Transaction transaction = session.beginTransaction();
StatementResult result = transaction.run( "CREATE (n {name:'Bob'})" );
Expand All @@ -276,41 +303,7 @@ void shouldThrowCorrectErrorOnCommitFailure() throws Throwable
}
}

private static void testTransactionCloseErrorPropagationWhenSessionClosed( String script, boolean commit,
String expectedErrorMessage ) throws Exception
{
StubServer server = StubServer.start( script, 9001 );
try
{
try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ) )
{
Session session = driver.session();

Transaction tx = session.beginTransaction();
StatementResult result = tx.run( "CREATE (n {name:'Alice'}) RETURN n.name AS name" );
assertEquals( "Alice", result.single().get( "name" ).asString() );

if ( commit )
{
tx.success();
}
else
{
tx.failure();
}

TransientException e = assertThrows( TransientException.class, session::close );
assertEquals( "Neo.TransientError.General.DatabaseUnavailable", e.code() );
assertEquals( expectedErrorMessage, e.getMessage() );
}
}
finally
{
assertEquals( 0, server.exitStatus() );
}
}

private static void testTxCloseErrorPropagation( String script, boolean commit, String expectedErrorMessage )
private static void testTxCloseErrorPropagation( String script, Consumer<Transaction> txAction, String expectedErrorMessage )
throws Exception
{
StubServer server = StubServer.start( script, 9001 );
Expand All @@ -323,16 +316,8 @@ private static void testTxCloseErrorPropagation( String script, boolean commit,
StatementResult result = tx.run( "CREATE (n {name:'Alice'}) RETURN n.name AS name" );
assertEquals( "Alice", result.single().get( "name" ).asString() );

if ( commit )
{
tx.success();
}
else
{
tx.failure();
}
TransientException e = assertThrows( TransientException.class, () -> txAction.accept( tx ) );

TransientException e = assertThrows( TransientException.class, tx::close );
assertEquals( "Neo.TransientError.General.DatabaseUnavailable", e.code() );
assertEquals( expectedErrorMessage, e.getMessage() );
}
Expand Down
Loading