You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2020/03/05 19:48:19 UTC
[tinkerpop] 03/04: TINKERPOP-2336 Removed maxWaitForSessionClose
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 89faefc50848e957348ee615d6c72496c0621b43
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Thu Mar 5 14:31:17 2020 -0500
TINKERPOP-2336 Removed maxWaitForSessionClose
This was replaced by the more robust maxWaitForClose setting in 3.3.11. Sending a close message to a session no longer has any effect. The channel must close to close the session. Prevent more than one client from connecting to the same session. CTR
---
CHANGELOG.asciidoc | 3 +
docs/src/dev/provider/index.asciidoc | 11 +--
docs/src/reference/gremlin-variants.asciidoc | 1 -
docs/src/upgrade/release-3.5.x.asciidoc | 22 ++++-
.../apache/tinkerpop/gremlin/driver/Cluster.java | 35 -------
.../tinkerpop/gremlin/driver/Connection.java | 36 -------
.../apache/tinkerpop/gremlin/driver/Settings.java | 19 +---
.../gremlin/driver/ClusterBuilderTest.java | 4 +-
.../server/op/session/SessionOpProcessor.java | 25 ++---
.../server/GremlinServerSessionIntegrateTest.java | 108 ++++++++-------------
10 files changed, 86 insertions(+), 178 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index c49b385..7e9fee5 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -46,9 +46,12 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>.
* Added a parameterized `TypeTranslator` for use with `GroovyTranslator` that should produce more cache hits.
* Added support for `TextP` in Neo4j using its string search functions.
* Changed `TraversalStrategy` application methodology to apply each strategy in turn to each level of the traversal hierarchy starting from root down to children.
+* Prevented more than one `Client` from connecting to the same Gremlin Server session.
+* Removed internal functionality for the session close message in Gremlin Server - the message is accepted but ignored if sent.
* Removed `Property.Exceptions.propertyValueCanNotBeNull` exception type as `null` now has meaning in Gremlin.
* Removed the "experimental" support for multi/meta-properties in Neo4j.
* Removed Gryo serialization configurations from Gremlin Server sample configurations and default configurations.
+* Removed previously deprecated `Cluster.maxWaitForSessionClose` configuration option.
* Removed previously deprecated `TraversalStrategies.applyStrategies()`.
* Removed previously deprecated `scriptEvaluationTimeout`.
* Removed previously deprecated `NioChannelizer` and related classes.
diff --git a/docs/src/dev/provider/index.asciidoc b/docs/src/dev/provider/index.asciidoc
index 47f010c..9d67394 100644
--- a/docs/src/dev/provider/index.asciidoc
+++ b/docs/src/dev/provider/index.asciidoc
@@ -1023,14 +1023,13 @@ Gremlin Server are deployed, session state is not shared among them.
!Key !Description
!`authentication` !A request that contains the response to a server challenge for authentication.
!`eval` !Evaluate a Gremlin script provided as a `String`.
-!`close` !Close the specified session. Will return a `NO CONTENT` message as confirmation of the close being completed.
|=========================================================
-NOTE: The "close" message is deprecated as of 3.3.11 as servers at this version are required to automatically interrupt
-running processes on the close of the connection and release resources such as sessions. Servers wishing to be
-compatible with older versions of the driver need only send back a `NO_CONTENT` for this message. Drivers wishing to
-be compatible with servers prior to 3.3.11 may continue to send the message on calls to `close()` otherwise such code
-can be removed.
+NOTE: There was a "close" message related to sessions that was deprecated as of 3.3.11. It's functionality was removed
+in 3.5.0. Servers wishing to be compatible with older versions of the driver need only send back a `NO_CONTENT` for
+this message (which is what Gremlin Server does as of 3.5.0). Drivers wishing to be compatible with servers prior to
+3.3.11 may continue to send the message on calls to `close()` (TinkerPop drivers no longer do that as of 3.5.0)
+otherwise such code can be removed.
**`authentication` operation arguments**
diff --git a/docs/src/reference/gremlin-variants.asciidoc b/docs/src/reference/gremlin-variants.asciidoc
index 68a3083..f7c8b26 100644
--- a/docs/src/reference/gremlin-variants.asciidoc
+++ b/docs/src/reference/gremlin-variants.asciidoc
@@ -256,7 +256,6 @@ The following table describes the various configuration options for the Gremlin
|connectionPool.maxSimultaneousUsagePerConnection |The maximum number of times that a connection can be borrowed from the pool simultaneously. |16
|connectionPool.maxSize |The maximum size of a connection pool for a host. |8
|connectionPool.maxWaitForConnection |The amount of time in milliseconds to wait for a new connection before timing out. |3000
-|connectionPool.maxWaitForSessionClose |The amount of time in milliseconds to wait for a session to close before timing out (does not apply to sessionless connections). |3000
|connectionPool.minInProcessPerConnection |The minimum number of in-flight requests that can occur on a connection. |1
|connectionPool.minSimultaneousUsagePerConnection |The maximum number of times that a connection can be borrowed from the pool simultaneously. |8
|connectionPool.minSize |The minimum size of a connection pool for a host. |2
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index 9c50a0c..5c163ce 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -255,6 +255,7 @@ The following deprecated classes, methods or fields have been removed in this ve
** `org.apache.tinkerpop.gremlin.driver.Cluster.Builder#keyCertChainFile(String)`
** `org.apache.tinkerpop.gremlin.driver.Cluster.Builder#keyFile(String)`
** `org.apache.tinkerpop.gremlin.driver.Cluster.Builder#keyPassword(String)`
+** `org.apache.tinkerpop.gremlin.driver.Cluster.Builder#maxWaitForSessionClose(Integer)`
** `org.apache.tinkerpop.gremlin.driver.Cluster.Builder#trustCertificateChainFile(String)`
** `org.apache.tinkerpop.gremlin.driver.handler.NioGremlinRequestEncoder`
** `org.apache.tinkerpop.gremlin.driver.handler.NioGremlinResponseDecoder`
@@ -580,6 +581,16 @@ that rely on the old step names.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-2254[TINKERPOP-2254]
+===== Session Close
+
+TinkerPop drivers no longer send the session "close" message to kill a session. The close of the connection itself
+should be responsible for the close of the session. It is also expected that a session is bound to the client that
+created it. Closing the session explicitly by closing the connection will act as a force close where transaction are
+not explicitly rolled-back by Gremlin Server. Such transactions would be handled by the underlying graph system in the
+manner that they provide.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2336[TINKERPOP-2336]
+
==== Graph Driver Providers
===== TraversalOpProcessor Side-effects
@@ -587,4 +598,13 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-2254[TINKERPOP-2254]
`TraversalOpProcessor` no longer holds a cache of side-effects and more generally the entire side-effect protocol has
been removed and is no longer supported in the server or drivers.
-See: link:https://issues.apache.org/jira/browse/TINKERPOP-2269[TINKERPOP-2269]
\ No newline at end of file
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2269[TINKERPOP-2269]
+
+===== Close Message
+
+The functionality of the "close" message is no longer in place in Gremlin Server. Sending the message (from older
+drivers for example) will simply result in a no-op on the server and the expected return of the `NO_CONTENT` message.
+From 3.5.0 forward, drivers need not send this message to close the session and simply rely on the close of the
+connection to kill the session.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2336[TINKERPOP-2336]
\ No newline at end of file
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Cluster.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Cluster.java
index 15d5acc..c962ddb 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Cluster.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Cluster.java
@@ -394,16 +394,6 @@ public final class Cluster {
}
/**
- * Gets how long a session will stay open assuming the current connection actually is configured for their use.
- *
- * @deprecated As of release 3.3.11, replaced in essence by {@link #getMaxWaitForClose()}.
- */
- @Deprecated
- public int getMaxWaitForSessionClose() {
- return manager.connectionPoolSettings.maxWaitForSessionClose;
- }
-
- /**
* Gets how long a connection will wait for all pending messages to be returned from the server before closing.
*/
public int getMaxWaitForClose() {
@@ -567,7 +557,6 @@ public final class Cluster {
private int maxInProcessPerConnection = Connection.MAX_IN_PROCESS;
private int minInProcessPerConnection = Connection.MIN_IN_PROCESS;
private int maxWaitForConnection = Connection.MAX_WAIT_FOR_CONNECTION;
- private int maxWaitForSessionClose = Connection.MAX_WAIT_FOR_SESSION_CLOSE;
private int maxWaitForClose = Connection.MAX_WAIT_FOR_CLOSE;
private int maxContentLength = Connection.MAX_CONTENT_LENGTH;
private int reconnectInterval = Connection.RECONNECT_INTERVAL;
@@ -837,29 +826,9 @@ public final class Cluster {
}
/**
- * If the connection is using a "session" this setting represents the amount of time in milliseconds to wait
- * for that session to close before timing out where the default value is 3000. Note that the server will
- * eventually clean up dead sessions itself on expiration of the session or during shutdown.
- *
- * @deprecated As of release 3.3.11, replaced in essence by {@link #maxWaitForClose(int)} though behavior
- * described here is still maintained.
- */
- @Deprecated
- public Builder maxWaitForSessionClose(final int maxWait) {
- this.maxWaitForSessionClose = maxWait;
- return this;
- }
-
- /**
* The amount of time in milliseconds to wait the connection to close before timing out where the default
* value is 3000. This timeout allows for a delay to occur in waiting for remaining messages that may still
* be returning from the server while a {@link Client#close()} is called.
- * <p/>
- * This setting is related to {@link #maxWaitForSessionClose} to some degree. This setting refers to a wait
- * for standard requests (i.e. queries) but the {@link #maxWaitForSessionClose} refers to a wait for the
- * "session close" message that occurs after all standard requests have returned (or timed out). There is
- * generally no need to set {@link #maxWaitForSessionClose} if the server is on 3.3.11 or later as the close
- * of the connection will trigger the close of the session and the release of resources.
*/
public Builder maxWaitForClose(final int maxWait) {
this.maxWaitForClose = maxWait;
@@ -1050,7 +1019,6 @@ public final class Cluster {
connectionPoolSettings.maxSize = builder.maxConnectionPoolSize;
connectionPoolSettings.minSize = builder.minConnectionPoolSize;
connectionPoolSettings.maxWaitForConnection = builder.maxWaitForConnection;
- connectionPoolSettings.maxWaitForSessionClose = builder.maxWaitForSessionClose;
connectionPoolSettings.maxWaitForClose = builder.maxWaitForClose;
connectionPoolSettings.maxContentLength = builder.maxContentLength;
connectionPoolSettings.reconnectInterval = builder.reconnectInterval;
@@ -1117,9 +1085,6 @@ public final class Cluster {
if (builder.maxWaitForConnection < 1)
throw new IllegalArgumentException("maxWaitForConnection must be greater than zero");
- if (builder.maxWaitForSessionClose < 1)
- throw new IllegalArgumentException("maxWaitForSessionClose must be greater than zero");
-
if (builder.maxWaitForClose < 1)
throw new IllegalArgumentException("maxWaitForClose must be greater than zero");
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
index 50738d9..bcacf69 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
@@ -62,7 +62,6 @@ final class Connection {
public static final int MAX_IN_PROCESS = 4;
public static final int MIN_IN_PROCESS = 1;
public static final int MAX_WAIT_FOR_CONNECTION = 16000;
- public static final int MAX_WAIT_FOR_SESSION_CLOSE = 3000;
public static final int MAX_WAIT_FOR_CLOSE = 3000;
public static final int MAX_CONTENT_LENGTH = 65536;
@@ -311,41 +310,6 @@ final class Connection {
// messages at the server and leads to ugly log messages over there.
if (shutdownInitiated.compareAndSet(false, true)) {
final String connectionInfo = this.getConnectionInfo();
-
- // this block of code that "closes" the session is deprecated as of 3.3.11 - this message is going to be
- // removed at 3.5.0. we will instead bind session closing to the close of the channel itself and not have
- // this secondary operation here which really only acts as a means for clearing resources in a functioning
- // session. "functioning" in this context means that the session is not locked up with a long running
- // operation which will delay this close execution which ideally should be more immediate, as in the user
- // is annoyed that a long run operation is happening and they want an immediate cancellation. that's the
- // most likely use case. we also get the nice benefit that this if/then code just goes away as the
- // Connection really shouldn't care about the specific Client implementation.
- if (client instanceof Client.SessionedClient) {
- final boolean forceClose = client.getSettings().getSession().get().isForceClosed();
- final RequestMessage closeMessage = client.buildMessage(
- RequestMessage.build(Tokens.OPS_CLOSE).addArg(Tokens.ARGS_FORCE, forceClose)).create();
-
- final CompletableFuture<ResultSet> closed = new CompletableFuture<>();
- write(closeMessage, closed);
-
- try {
- // make sure we get a response here to validate that things closed as expected. on error, we'll let
- // the server try to clean up on its own. the primary error here should probably be related to
- // protocol issues which should not be something a user has to fuss with.
- closed.join().all().get(cluster.getMaxWaitForSessionClose(), TimeUnit.MILLISECONDS);
- } catch (TimeoutException ex) {
- final String msg = String.format(
- "Timeout while trying to close connection on %s - force closing - server will close session on shutdown or expiration.",
- ((Client.SessionedClient) client).getSessionId());
- logger.warn(msg, ex);
- } catch (Exception ex) {
- final String msg = String.format(
- "Encountered an error trying to close connection on %s - force closing - server will close session on shutdown or expiration.",
- ((Client.SessionedClient) client).getSessionId());
- logger.warn(msg, ex);
- }
- }
-
channelizer.close(channel);
final ChannelPromise promise = channel.newPromise();
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Settings.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Settings.java
index 2ca823a..5f3424d 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Settings.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Settings.java
@@ -156,9 +156,7 @@ final class Settings {
final Configuration serializerConfigConf = conf.subset("serializer.config");
if (IteratorUtils.count(serializerConfigConf.getKeys()) > 0) {
final Map<String,Object> m = new HashMap<>();
- serializerConfigConf.getKeys().forEachRemaining(name -> {
- m.put(name, serializerConfigConf.getProperty(name));
- });
+ serializerConfigConf.getKeys().forEachRemaining(name -> m.put(name, serializerConfigConf.getProperty(name)));
serializerSettings.config = m;
}
settings.serializer = serializerSettings;
@@ -224,9 +222,6 @@ final class Settings {
if (connectionPoolConf.containsKey("maxWaitForConnection"))
cpSettings.maxWaitForConnection = connectionPoolConf.getInt("maxWaitForConnection");
- if (connectionPoolConf.containsKey("maxWaitForSessionClose"))
- cpSettings.maxWaitForSessionClose = connectionPoolConf.getInt("maxWaitForSessionClose");
-
if (connectionPoolConf.containsKey("maxWaitForClose"))
cpSettings.maxWaitForClose = connectionPoolConf.getInt("maxWaitForClose");
@@ -361,16 +356,6 @@ final class Settings {
public int maxWaitForConnection = Connection.MAX_WAIT_FOR_CONNECTION;
/**
- * If the connection is using a "session" this setting represents the amount of time in milliseconds to wait
- * for that session to close before timing out where the default value is 3000. Note that the server will
- * eventually clean up dead sessions itself on expiration of the session or during shutdown.
- *
- * @deprecated As of release 3.3.11, replaced in essence by {@link #maxWaitForClose}.
- */
- @Deprecated
- public int maxWaitForSessionClose = Connection.MAX_WAIT_FOR_SESSION_CLOSE;
-
- /**
* The amount of time in milliseconds to wait the connection to close before timing out where the default
* value is 3000. This timeout allows for a delay to occur in waiting for remaining messages that may still
* be returning from the server while a {@link Client#close()} is called.
@@ -422,7 +407,7 @@ final class Settings {
public Map<String, Object> config = null;
public MessageSerializer create() throws Exception {
- final Class clazz = Class.forName(className);
+ final Class<?> clazz = Class.forName(className);
final MessageSerializer serializer = (MessageSerializer) clazz.newInstance();
Optional.ofNullable(config).ifPresent(c -> serializer.configure(c, null));
return serializer;
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterBuilderTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterBuilderTest.java
index 0bf317d..e494397 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterBuilderTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterBuilderTest.java
@@ -52,8 +52,8 @@ public class ClusterBuilderTest {
{"minConnectionPoolSizeLteMax", Cluster.build().minConnectionPoolSize(100).maxConnectionPoolSize(99), "maxConnectionPoolSize cannot be less than minConnectionPoolSize"},
{"minConnectionPoolSizeLteMax", Cluster.build().minConnectionPoolSize(100).maxConnectionPoolSize(99), "maxConnectionPoolSize cannot be less than minConnectionPoolSize"},
{"maxConnectionPoolSize0", Cluster.build().maxWaitForConnection(0), "maxWaitForConnection must be greater than zero"},
- {"maxWaitForSessionClose0", Cluster.build().maxWaitForSessionClose(0), "maxWaitForSessionClose must be greater than zero"},
- {"maxWaitForSessionCloseNeg1", Cluster.build().maxWaitForSessionClose(-1), "maxWaitForSessionClose must be greater than zero"},
+ {"maxWaitForClose0", Cluster.build().maxWaitForClose(0), "maxWaitForClose must be greater than zero"},
+ {"maxWaitForCloseNeg1", Cluster.build().maxWaitForClose(-1), "maxWaitForClose must be greater than zero"},
{"maxContentLength0", Cluster.build().maxContentLength(0), "maxContentLength must be greater than zero"},
{"maxContentLengthNeg1", Cluster.build().maxContentLength(-1), "maxContentLength must be greater than zero"},
{"reconnectInterval0", Cluster.build().reconnectInterval(0), "reconnectInterval must be greater than zero"},
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
index 2bcb430..b4d6d4e 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
@@ -122,13 +122,14 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
}
/**
- * Session based requests accept a "close" operator in addition to "eval". A close will trigger the session to be
- * killed and any uncommitted transaction to be rolled-back.
+ * Older versions of session-based requests accepted a "close" operator in addition to "eval". This feature
+ * effectively acts as a do-nothing for 3.5.0 to allow older versions of the drivers to connect. At some point
+ * this may be removed completely. Note that closing the channel kills the session now.
*/
@Override
public Optional<ThrowingConsumer<Context>> selectOther(final RequestMessage requestMessage) throws OpProcessorException {
- // deprecated the "close" message at 3.3.11 - should probably leave this check for the "close" token so that
- // if older versions of the driver connect they won't get an error. can basically just write back a NO_CONTENT
+ // deprecated the "close" message at 3.3.11 - left this check for the "close" token so that if older versions
+ // of the driver connect they won't get an error. basically just writes back a NO_CONTENT
// for the immediate term in 3.5.0 and then for some future version remove support for the message completely
// and thus disallow older driver versions from connecting at all.
if (requestMessage.getOp().equals(Tokens.OPS_CLOSE)) {
@@ -138,14 +139,7 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
throw new OpProcessorException(msg, ResponseMessage.build(requestMessage).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create());
}
- final boolean force = requestMessage.<Boolean>optionalArgs(Tokens.ARGS_FORCE).orElse(false);
-
return Optional.of(rhc -> {
- final Session sessionToClose = sessions.get(requestMessage.getArgs().get(Tokens.ARGS_SESSION).toString());
- if (null != sessionToClose) {
- sessionToClose.manualKill(force);
- }
-
// send back a confirmation of the close
rhc.writeAndFlush(ResponseMessage.build(requestMessage)
.code(ResponseStatusCode.NO_CONTENT)
@@ -191,6 +185,15 @@ public class SessionOpProcessor extends AbstractEvalOpProcessor {
throw new OpProcessorException(sessionClosedMessage, response);
}
+ // check if the session is bound to this channel, thus one client per session
+ if (!session.isBoundTo(context.getChannelHandlerContext().channel())) {
+ final String sessionClosedMessage = String.format("Session %s is not bound to the connecting client",
+ session.getSessionId());
+ final ResponseMessage response = ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR)
+ .statusMessage(sessionClosedMessage).create();
+ throw new OpProcessorException(sessionClosedMessage, response);
+ }
+
// place the session on the channel context so that it can be used during serialization. in this way
// the serialization can occur on the same thread used to execute the gremlin within the session. this
// is important given the threadlocal nature of Graph implementation transactions.
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSessionIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSessionIntegrateTest.java
index 0c76b8d..7c00bb5 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSessionIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSessionIntegrateTest.java
@@ -49,7 +49,7 @@ import java.util.stream.IntStream;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.core.IsCollectionContaining.hasItem;
+import static org.hamcrest.core.IsIterableContaining.hasItem;
import static org.hamcrest.core.StringStartsWith.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -93,8 +93,7 @@ public class GremlinServerSessionIntegrateTest extends AbstractGremlinServerInt
settings.processors.add(processorSettings);
Logger.getRootLogger().setLevel(Level.INFO);
break;
- case "shouldBlockAdditionalRequestsDuringClose":
- case "shouldBlockAdditionalRequestsDuringForceClose":
+ case "shouldCloseSessionOnClientClose":
clearNeo4j(settings);
Logger.getRootLogger().setLevel(Level.INFO);
break;
@@ -125,6 +124,38 @@ public class GremlinServerSessionIntegrateTest extends AbstractGremlinServerInt
}
@Test
+ public void shouldCloseSessionOnClientClose() throws Exception {
+ assumeNeo4jIsPresent();
+
+ final Cluster cluster1 = TestClientFactory.open();
+ final Client client1 = cluster1.connect(name.getMethodName());
+ client1.submit("x = 1").all().join();
+ client1.submit("graph.addVertex()").all().join();
+ client1.close();
+ cluster1.close();
+
+ assertThat(recordingAppender.getMessages(), hasItem("INFO - Skipped attempt to close open graph transactions on shouldCloseSessionOnClientClose - close was forced\n"));
+ assertThat(recordingAppender.getMessages(), hasItem("INFO - Session shouldCloseSessionOnClientClose closed\n"));
+
+ // try to reconnect to that session and make sure no state is there
+ final Cluster clusterReconnect = TestClientFactory.open();
+ final Client clientReconnect = clusterReconnect.connect(name.getMethodName());
+
+ // should get an error because "x" is not defined as this is a new session
+ try {
+ clientReconnect.submit("x").all().join();
+ fail("Should not have been successful as 'x' was only defined in the old session");
+ } catch(Exception ex) {
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertThat(root.getMessage(), startsWith("No such property"));
+ }
+
+ // the commit from client1 should not have gone through so there should be no data present.
+ assertEquals(0, clientReconnect.submit("graph.traversal().V().count()").all().join().get(0).getInt());
+ clusterReconnect.close();
+ }
+
+ @Test
public void shouldUseGlobalFunctionCache() throws Exception {
final Cluster cluster = TestClientFactory.open();
final Client client = cluster.connect(name.getMethodName());
@@ -161,85 +192,24 @@ public class GremlinServerSessionIntegrateTest extends AbstractGremlinServerInt
}
@Test
- public void shouldBlockAdditionalRequestsDuringClose() throws Exception {
- assumeNeo4jIsPresent();
-
- // this is sorta cobbled together a bit given limits/rules about how you can use Cluster/Client instances.
- // basically, we need one to submit the long run job and one to do the close operation that will cancel the
- // long run job. it is probably possible to do this with some low-level message manipulation but that's
- // probably not necessary
- //
- // this test wont work so well once we remove the sending of the session close message from the driver which
- // got deprecated at 3.3.11 and lock a session to the connection that created it. in that case, two Client
- // instances won't be able to connect to the same session which is what is happening below. not sure what
- // form this test should take then especially since transactions will force close when the channel closes.
- // perhaps it should just be removed.
- final Cluster cluster1 = TestClientFactory.open();
- final Client client1 = cluster1.connect(name.getMethodName());
- client1.submit("graph.addVertex()").all().join();
- final Cluster cluster2 = TestClientFactory.open();
- final Client client2 = cluster2.connect(name.getMethodName());
- client2.submit("1+1").all().join();
-
- final ResultSet rs = client1.submit("Thread.sleep(3000);1+1");
-
- // close while the previous request is still executing
- client2.close();
-
- assertEquals(2, rs.all().join().get(0).getInt());
-
- client1.close();
-
- cluster1.close();
- cluster2.close();
-
- // triggered an error during close and since we didn't force close, the attempt to close the transaction
- // is made
- assertThat(recordingAppender.getMessages(), hasItem("INFO - Rolling back open transactions on graph before killing session: " + name.getMethodName() + "\n"));
-
- }
-
- @Test
- public void shouldBlockAdditionalRequestsDuringForceClose() throws Exception {
- assumeNeo4jIsPresent();
-
- // this is sorta cobbled together a bit given limits/rules about how you can use Cluster/Client instances.
- // basically, we need one to submit the long run job and one to do the close operation that will cancel the
- // long run job. it is probably possible to do this with some low-level message manipulation but that's
- // probably not necessary
- //
- // this test wont work so well once we remove the sending of the session close message from the driver which
- // got deprecated at 3.3.11 and lock a session to the connection that created it. in that case, two Client
- // instances won't be able to connect to the same session which is what is happening below. not sure what
- // form this test should take then especially since transactions will force close when the channel closes.
- // perhaps it should just be removed.
+ public void shouldNotAllowMoreThanOneClientPerSession() throws Exception {
final Cluster cluster1 = TestClientFactory.open();
final Client client1 = cluster1.connect(name.getMethodName());
- client1.submit("graph.addVertex()").all().join();
+ client1.submit("1+1").all().join();
final Cluster cluster2 = TestClientFactory.open();
final Client.SessionSettings sessionSettings = Client.SessionSettings.build()
.sessionId(name.getMethodName())
.forceClosed(true).create();
final Client client2 = cluster2.connect(Client.Settings.build().useSession(sessionSettings).create());
- client2.submit("1+1").all().join();
-
- final ResultSet rs = client1.submit("Thread.sleep(10000);1+1");
-
- client2.close();
-
- // because the close was forced, the message should appear immediately
- assertThat(recordingAppender.getMessages(), hasItem("INFO - Skipped attempt to close open graph transactions on " + name.getMethodName() + " - close was forced\n"));
try {
- rs.all().join();
- fail("The close of the session on client2 should have interrupted the script sent on client1");
+ client2.submit("2+2").all().join();
+ fail("Can't have more than one client connecting to the same session");
} catch (Exception ex) {
final Throwable root = ExceptionUtils.getRootCause(ex);
- assertThat(root.getMessage(), startsWith("Evaluation exceeded the configured 'evaluationTimeout' threshold of 30000 ms or evaluation was otherwise cancelled directly for request"));
+ assertEquals("Session shouldNotAllowMoreThanOneClientPerSession is not bound to the connecting client", root.getMessage());
}
- client1.close();
-
cluster1.close();
cluster2.close();
}