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 2021/04/02 12:34:21 UTC
[tinkerpop] 04/06: Closing the alias needs to close the underlying
client with it
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2245
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 7b92c4c69350fa00f98d2b73c06c9a92a76ee4aa
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Thu Apr 1 11:00:44 2021 -0400
Closing the alias needs to close the underlying client with it
This is a bit of a change in behavior but it's a necessary one. Perhaps aliasing should be refactored at some point to not need AliasClusteredClient as a proxy. The Client itself should probably just be configured with the required alias. CTR
---
CHANGELOG.asciidoc | 3 +-
.../apache/tinkerpop/gremlin/driver/Client.java | 16 +++++-----
.../gremlin/server/GremlinDriverIntegrateTest.java | 10 +++----
.../server/GremlinSessionTxIntegrateTest.java | 35 +++++++++++++++++++++-
4 files changed, 49 insertions(+), 15 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 041c278..37da741 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -23,7 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-3-5-0]]
=== TinkerPop 3.5.0 (Release Date: NOT OFFICIALLY RELEASED YET)
-This release also includes changes from <<release-3-4-3, 3.4.3>>.
+This release also includes changes from <<release-3-4-11, 3.4.11>>.
* Added `gremlin-language` module.
* Allowed the possibility for the propagation of `null` as a `Traverser` in Gremlin.
@@ -41,6 +41,7 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>.
* Allowed additional arguments to `Client.submit()` in Javascript driver to enable setting of parameters like `scriptEvaluationTimeout`.
* Gremlin.Net driver no longer supports skipping deserialization by default. Users can however create their own `IMessageSerializer` if they need this functionality.
* Supported deserialization of `dict` and `list` as a key in a `dict` for Python.
+* Changed the aliased `Client` to proxy `close()` methods to its underlying client.
* Added support for remote `g.tx()` usage.
* Added support for bytecode-based sessions.
* Added a `Graph.Feature` for `supportsNullPropertyValues`.
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
index 3ad22cf..4a38951 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
@@ -360,7 +360,7 @@ public abstract class Client {
* A low-level method that allows the submission of a manually constructed {@link RequestMessage}.
*/
public CompletableFuture<ResultSet> submitAsync(final RequestMessage msg) {
- if (isClosing()) throw new IllegalStateException("Client has been closed");
+ if (isClosing()) throw new IllegalStateException("Client is closed");
if (!initialized)
init();
@@ -655,19 +655,19 @@ public abstract class Client {
return client.chooseConnection(msg);
}
- /**
- * Prevents messages from being sent from this {@code Client}. Note that calling this method does not call
- * close on the {@code Client} that created it.
- */
+ @Override
+ public void close() {
+ client.close();
+ }
+
@Override
public synchronized CompletableFuture<Void> closeAsync() {
- close.complete(null);
- return close;
+ return client.closeAsync();
}
@Override
public boolean isClosing() {
- return close.isDone();
+ return client.isClosing();
}
/**
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
index f006cc4..7e09c95 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
@@ -1702,7 +1702,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
} catch (Exception ex) {
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(IllegalStateException.class));
- assertEquals("Client has been closed", root.getMessage());
+ assertEquals("Client is closed", root.getMessage());
}
try {
@@ -1711,7 +1711,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
} catch (Exception ex) {
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(IllegalStateException.class));
- assertEquals("Client has been closed", root.getMessage());
+ assertEquals("Client is closed", root.getMessage());
}
try {
@@ -1720,7 +1720,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
} catch (Exception ex) {
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(IllegalStateException.class));
- assertEquals("Client has been closed", root.getMessage());
+ assertEquals("Client is closed", root.getMessage());
}
try {
@@ -1729,7 +1729,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
} catch (Exception ex) {
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(IllegalStateException.class));
- assertEquals("Client has been closed", root.getMessage());
+ assertEquals("Client is closed", root.getMessage());
}
try {
@@ -1738,7 +1738,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
} catch (Exception ex) {
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(IllegalStateException.class));
- assertEquals("Client has been closed", root.getMessage());
+ assertEquals("Client is closed", root.getMessage());
}
// allow call to close() even though closed through cluster
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinSessionTxIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinSessionTxIntegrateTest.java
index ae8817f..534f867 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinSessionTxIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinSessionTxIntegrateTest.java
@@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.driver.Cluster;
import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.structure.Transaction;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.junit.Test;
import java.io.File;
@@ -188,7 +189,7 @@ public class GremlinSessionTxIntegrateTest extends AbstractGremlinServerIntegrat
gtx.addV("person").iterate();
assertEquals(1, (long) gtx.V().count().next());
- gtx.tx().close();
+ gtx.close();
assertThat(gtx.tx().isOpen(), is(false));
// sessionless connections should still be good - close() should not affect that
@@ -207,6 +208,38 @@ public class GremlinSessionTxIntegrateTest extends AbstractGremlinServerIntegrat
}
@Test
+ public void shouldOpenAndCloseObsceneAmountOfSessions() throws Exception {
+ assumeNeo4jIsPresent();
+
+ final Cluster cluster = TestClientFactory.build().create();
+ final GraphTraversalSource g = traversal().withRemote(DriverRemoteConnection.using(cluster));
+
+ // need to open significantly more sessions that we have threads in gremlinPool
+ final int numberOfSessions = 500;
+ for (int ix = 0; ix < numberOfSessions; ix ++) {
+ final Transaction tx = g.tx();
+ final GraphTraversalSource gtx = tx.begin();
+ try {
+ final Vertex v1 = gtx.addV("person").property("pid", ix + "a").next();
+ final Vertex v2 = gtx.addV("person").property("pid", ix + "b").next();
+ gtx.addE("knows").from(v1).to(v2).iterate();
+ tx.commit();
+ } catch (Exception ex) {
+ tx.rollback();
+ fail("Should not expect any failures");
+ } finally {
+ assertThat(tx.isOpen(), is(false));
+ }
+ }
+
+ // sessionless connections should still be good - close() should not affect that
+ assertEquals(numberOfSessions * 2, (long) g.V().count().next());
+ assertEquals(numberOfSessions, (long) g.E().count().next());
+
+ cluster.close();
+ }
+
+ @Test
public void shouldCommitTxBytecodeInSessionReusingGtxAcrossThreads() throws Exception {
assumeNeo4jIsPresent();