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 2016/09/25 02:22:08 UTC
tinkerpop git commit: TraversalSource and Traversal implement
AutoCloseable.
Repository: tinkerpop
Updated Branches:
refs/heads/TINKERPOP-790 [created] 14211691e
TraversalSource and Traversal implement AutoCloseable.
This allowed for TraversalSource instances created using withRemote() to close out open RemoteConnection instances. It also sets up for side-effects created by a "remote" Traversal to be cleared from cache on the server. That work is not done yet and is reserved for a different ticket.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/14211691
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/14211691
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/14211691
Branch: refs/heads/TINKERPOP-790
Commit: 14211691effffec1f9fdf6ddd92c5833dff234de
Parents: 817961f
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Sat Sep 24 22:18:56 2016 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Sat Sep 24 22:18:56 2016 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../src/reference/gremlin-applications.asciidoc | 10 +++-
.../upgrade/release-3.2.x-incubating.asciidoc | 25 +++++++++
.../traversal/RemoteTraversalSideEffects.java | 6 ++-
.../gremlin/process/traversal/Traversal.java | 7 ++-
.../process/traversal/TraversalSource.java | 15 +++---
.../dsl/graph/GraphTraversalSource.java | 24 +++++++--
.../dsl/graph/GraphTraversalSourceTest.java | 53 ++++++++++++++++++++
.../driver/remote/DriverRemoteTraversal.java | 13 +++++
.../DriverRemoteTraversalSideEffects.java | 7 +++
10 files changed, 147 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index a1b405f..1272598 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -34,6 +34,7 @@ TinkerPop 3.2.3 (Release Date: NOT OFFICIALLY RELEASED YET)
* `TraversalRing` returns a `null` if it does not contain traversals (previously `IdentityTraversal`).
* Fixed a `JavaTranslator` bug where `Bytecode` instructions were being mutated during translation.
* Added `Path` to Gremlin-Python with respective GraphSON 2.0 deserializer.
+* `Traversal` and `TraversalSource` now implement `AutoCloseable`.
* Renamed the `empty.result.indicator` preference to `result.indicator.null` in Gremlin Console
* If `result.indicator.null` is set to an empty string, then no "result line" is printed in Gremlin Console.
* VertexPrograms can now declare traverser requirements, e.g. to have access to the path when used with `.program()`.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/docs/src/reference/gremlin-applications.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc
index 2c093fa..8a3a9f7 100644
--- a/docs/src/reference/gremlin-applications.asciidoc
+++ b/docs/src/reference/gremlin-applications.asciidoc
@@ -906,8 +906,13 @@ the `TraversalSource` be generated with `EmptyGraph` as follows:
graph = EmptyGraph.instance()
g = graph.traversal().withRemote('conf/remote-graph.properties')
g.V().valueMap(true)
+g.close()
----
+Note the call to `close()` above. The call to `withRemote()` internally instantiates a `Client` instance that can only
+be released by "closing" the `GraphTraversalSource`. It is important to take that step to release resources created
+in that step.
+
If working with multiple remote `TraversalSource` instances it is more efficient to construct a `Cluster` object and
then re-use it.
@@ -917,11 +922,14 @@ cluster = Cluster.open('conf/remote-objects.yaml')
graph = EmptyGraph.instance()
g = graph.traversal().withRemote(DriverRemoteConnection.using(cluster, "g"))
g.V().valueMap(true)
+g.close()
cluster.close()
----
If the `Cluster` instance is supplied externally, as is shown above, then it is not closed implicitly by the close of
-the `RemoteGraph`. In this case, the `Cluster` must be closed explicitly.
+"g". Closing "g" will only close the `Client` instance associated with that `TraversalSource`. In this case, the
+`Cluster` must also be closed explicitly. Closing "g" and the "cluster" aren't actually both necessary - the close of
+a `Cluster` will close all `Client` instance spawned by the `Cluster`.
IMPORTANT: `RemoteGraph` uses the `TraversalOpProcessor` in Gremlin Server which requires a cache to enable the
retrieval of side-effects (if the `Traversal` produces any). That cache can be configured (e.g. controlling eviction
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/docs/src/upgrade/release-3.2.x-incubating.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
index f9c62e2..af3fa22 100644
--- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
@@ -108,6 +108,31 @@ gremlin> g.V().hasLabel("software").count()
TinkerPop 3.2.3 fixes this misbehavior and all `has()` method overloads behave like before, except that they no longer
support no arguments.
+TraversalSource.close()
+^^^^^^^^^^^^^^^^^^^^^^^
+
+`TraversalSource` now implements `AutoCloseable`, which means that the `close()` method is now available. This new
+method is important in cases where `withRemote()` is used, as `withRemote()` can open "expensive" resources that need
+to be released.
+
+In the case of TinkerPop's `DriverRemoteConnection`, `close()` will destroy the `Client` instance that is created
+internally by `withRemote()` as shown below:
+
+[source,text]
+----
+gremlin> graph = EmptyGraph.instance()
+==>emptygraph[empty]
+gremlin> g = graph.traversal().withRemote('conf/remote-graph.properties')
+==>graphtraversalsource[emptygraph[empty], standard]
+gremlin> g.close()
+gremlin>
+----
+
+Note that the `withRemote()` method will call `close()` on a `RemoteConnection` passed directly to it as well, so
+there is no need to do that manually.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-790[TINKERPOP-790]
+
TinkerPop 3.2.2
---------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/RemoteTraversalSideEffects.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/RemoteTraversalSideEffects.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/RemoteTraversalSideEffects.java
index e41a42d..97fb36d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/RemoteTraversalSideEffects.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/RemoteTraversalSideEffects.java
@@ -23,5 +23,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalSideEffects;
/**
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
-public interface RemoteTraversalSideEffects extends TraversalSideEffects {
+public interface RemoteTraversalSideEffects extends TraversalSideEffects, AutoCloseable {
+ @Override
+ public default void close() throws Exception {
+ // do nothing
+ }
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
index eec9d3f..edc18e0 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
@@ -57,7 +57,7 @@ import java.util.stream.StreamSupport;
*
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
-public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
+public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, AutoCloseable {
public static class Symbols {
private Symbols() {
@@ -231,6 +231,11 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
}
}
+ @Override
+ public default void close() throws Exception {
+ // do nothing by default
+ }
+
/**
* A collection of {@link Exception} types associated with Traversal execution.
*/
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
index 7428d56..0d690e9 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
@@ -53,7 +53,7 @@ import java.util.function.UnaryOperator;
*
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
-public interface TraversalSource extends Cloneable {
+public interface TraversalSource extends Cloneable, AutoCloseable {
public static final String GREMLIN_REMOTE = "gremlin.remote.";
public static final String GREMLIN_REMOTE_CONNECTION_CLASS = GREMLIN_REMOTE + "remoteConnectionClass";
@@ -427,14 +427,12 @@ public interface TraversalSource extends Cloneable {
/**
* Configures the {@code TraversalSource} as a "remote" to issue the {@link Traversal} for execution elsewhere.
+ * Implementations should track {@link RemoteConnection} instances that are created and call
+ * {@link RemoteConnection#close()} on them when the {@code TraversalSource} itself is closed.
*
* @param connection the {@link RemoteConnection} instance to use to submit the {@link Traversal}.
*/
- public default TraversalSource withRemote(final RemoteConnection connection) {
- final TraversalSource clone = this.clone();
- clone.getStrategies().addStrategies(new RemoteStrategy(connection));
- return clone;
- }
+ public TraversalSource withRemote(final RemoteConnection connection);
public default Optional<Class> getAnonymousTraversalClass() {
return Optional.empty();
@@ -450,6 +448,11 @@ public interface TraversalSource extends Cloneable {
@SuppressWarnings("CloneDoesntDeclareCloneNotSupportedException")
public TraversalSource clone();
+ @Override
+ public default void close() throws Exception {
+ // do nothing
+ }
+
/**
* @deprecated As of release 3.2.0. Please use {@link Graph#traversal(Class)}.
*/
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
index fdafb57..0525f8d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
@@ -19,10 +19,10 @@
package org.apache.tinkerpop.gremlin.process.traversal.dsl.graph;
import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.PropertiesConfiguration;
import org.apache.tinkerpop.gremlin.process.computer.Computer;
import org.apache.tinkerpop.gremlin.process.computer.GraphComputer;
import org.apache.tinkerpop.gremlin.process.remote.RemoteConnection;
+import org.apache.tinkerpop.gremlin.process.remote.traversal.strategy.decoration.RemoteStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.Bindings;
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalEngine;
@@ -41,10 +41,8 @@ import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Transaction;
import org.apache.tinkerpop.gremlin.structure.Vertex;
-import org.apache.tinkerpop.gremlin.structure.util.GraphFactory;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
-import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
@@ -60,7 +58,7 @@ import java.util.function.UnaryOperator;
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
public class GraphTraversalSource implements TraversalSource {
-
+ protected transient RemoteConnection connection;
protected final Graph graph;
protected TraversalStrategies strategies;
protected Bytecode bytecode = new Bytecode();
@@ -242,7 +240,19 @@ public class GraphTraversalSource implements TraversalSource {
@Override
public GraphTraversalSource withRemote(final RemoteConnection connection) {
- return (GraphTraversalSource) TraversalSource.super.withRemote(connection);
+ try {
+ // check if someone called withRemote() more than once, so just release resources on the initial
+ // connection as you can't have more than one. maybe better to toss IllegalStateException??
+ if (this.connection != null)
+ this.connection.close();
+ } catch (Exception ignored) {
+ // not sure there's anything to do here
+ }
+
+ this.connection = connection;
+ final TraversalSource clone = this.clone();
+ clone.getStrategies().addStrategies(new RemoteStrategy(connection));
+ return (GraphTraversalSource) clone;
}
//// SPAWNS
@@ -309,6 +319,10 @@ public class GraphTraversalSource implements TraversalSource {
return this.graph.tx();
}
+ @Override
+ public void close() throws Exception {
+ if (connection != null) connection.close();
+ }
@Override
public String toString() {
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
new file mode 100644
index 0000000..f9cf1df
--- /dev/null
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSourceTest.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.process.traversal.dsl.graph;
+
+import org.apache.tinkerpop.gremlin.process.remote.RemoteConnection;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.junit.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+/**
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+public class GraphTraversalSourceTest {
+ @Test
+ public void shouldCloseRemoteConnectionOnWithRemote() throws Exception {
+ final RemoteConnection mock = mock(RemoteConnection.class);
+ final GraphTraversalSource g = EmptyGraph.instance().traversal().withRemote(mock);
+ g.close();
+
+ verify(mock, times(1)).close();
+ }
+
+ @Test
+ public void shouldNotAllowLeakRemoteConnectionsIfMultipleAreCreated() throws Exception {
+
+ final RemoteConnection mock1 = mock(RemoteConnection.class);
+ final RemoteConnection mock2 = mock(RemoteConnection.class);
+ final GraphTraversalSource g = EmptyGraph.instance().traversal().withRemote(mock1).withRemote(mock2);
+ g.close();
+
+ verify(mock1, times(1)).close();
+ verify(mock2, times(1)).close();
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java
index 71abc77..14d89cc 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java
@@ -46,6 +46,7 @@ public class DriverRemoteTraversal<S, E> extends AbstractRemoteTraversal<S, E> {
private final Iterator<Traverser.Admin<E>> traversers;
private Traverser.Admin<E> lastTraverser = EmptyTraverser.instance();
private final RemoteTraversalSideEffects sideEffects;
+ private final Client client;
public DriverRemoteTraversal(final ResultSet rs, final Client client, final boolean attach, final Optional<Configuration> conf) {
// attaching is really just for testing purposes. it doesn't make sense in any real-world scenario as it would
@@ -59,6 +60,7 @@ public class DriverRemoteTraversal<S, E> extends AbstractRemoteTraversal<S, E> {
this.traversers = new TraverserIterator<>(rs.iterator());
}
+ this.client = client;
this.sideEffects = new DriverRemoteTraversalSideEffects(
client,
rs.getOriginalRequestMessage().getRequestId(),
@@ -100,6 +102,17 @@ public class DriverRemoteTraversal<S, E> extends AbstractRemoteTraversal<S, E> {
}
}
+ /**
+ * Releases server-side resources related to this traversal (i.e. clearing the side-effect cache of data related to
+ * this traversal.
+ */
+ @Override
+ public void close() throws Exception {
+ sideEffects.close();
+
+ // leave the client open as it is owned by the DriverRemoteConnection not the traversal or side-effects
+ }
+
static class TraverserIterator<E> implements Iterator<Traverser.Admin<E>> {
private final Iterator<Result> inner;
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/14211691/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java
index bd51e67..85d7abc 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java
@@ -98,4 +98,11 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSid
return keys;
}
+
+ @Override
+ public void close() throws Exception {
+ // todo: need to add a call to "close" the side effects on the server - probably should ensure request only sends once
+
+ // leave the client open as it is owned by the DriverRemoteConnection not the traversal or side-effects
+ }
}