Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-1278 8dd021192 -> 688e3f551
TINKERPOP-1392 Remove support for Java serialized Traversal. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/688e3f55 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/688e3f55 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/688e3f55 Branch: refs/heads/TINKERPOP-1278 Commit: 688e3f551c260f86c87c55c48863052b2733a3ff Parents: 8dd0211 Author: Stephen Mallette <[email protected]> Authored: Mon Aug 1 13:09:24 2016 -0400 Committer: Stephen Mallette <[email protected]> Committed: Mon Aug 1 13:09:24 2016 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../upgrade/release-3.2.x-incubating.asciidoc | 20 +++ .../apache/tinkerpop/gremlin/driver/Client.java | 21 +--- .../driver/remote/DriverRemoteConnection.java | 25 +--- .../op/traversal/TraversalOpProcessor.java | 125 +------------------ .../server/GremlinResultSetIntegrateTest.java | 10 +- 6 files changed, 33 insertions(+), 169 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/688e3f55/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 2714469..25666dc 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.2 (NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Removed support for submitting a Java serialized `Traversal` to Gremlin Server. * Fixed a potential leak of a `ReferenceCounted` resource in Gremlin Server. * Added class registrations for `Map.Entry` implementations to `GryoMapper`. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/688e3f55/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 f85012c..fa55403 100644 --- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc +++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc @@ -22,6 +22,26 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima *Nine Inch Gremlins* +TinkerPop 3.2.2 +--------------- + +*Release Date: NOT OFFICIALLY RELEASED YET* + +Upgrading for Providers +~~~~~~~~~~~~~~~~~~~~~~~ + +Driver Providers +^^^^^^^^^^^^^^^^ + +Traversal Serialization ++++++++++++++++++++++++ + +There was an "internal" serialization format in place for `Traversal` which allowed one to be submitted to Gremlin +Server directly over `RemoteGraph`. That format has been removed completely and is wholly replaced by the non-JVM +specific approach of serializing `Bytecode`. + +See: link:https://issues.apache.org/jira/browse/TINKERPOP-1392[TINKERPOP-1392] + TinkerPop 3.2.1 --------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/688e3f55/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java ---------------------------------------------------------------------- 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 9b8f77f..3716411 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 @@ -26,7 +26,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.structure.Graph; import org.apache.tinkerpop.gremlin.structure.Vertex; -import org.apache.tinkerpop.gremlin.util.Serializer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -133,10 +132,7 @@ public abstract class Client { /** * Submit a {@link Traversal} to the server for remote execution. - * - * @deprecated As of release 3.2.2, replaced by {@link #submit(Bytecode)}. */ - @Deprecated public ResultSet submit(final Traversal traversal) { try { return submitAsync(traversal).get(); @@ -149,10 +145,7 @@ public abstract class Client { /** * An asynchronous version of {@link #submit(Traversal)}. - * - * @deprecated As of release 3.2.2, replaced by {@link #submitAsync(Bytecode)}. */ - @Deprecated public CompletableFuture<ResultSet> submitAsync(final Traversal traversal) { throw new UnsupportedOperationException("This implementation does not support Traversal submission - use a sessionless Client created with from the alias() method"); } @@ -564,19 +557,7 @@ public abstract class Client { @Override public CompletableFuture<ResultSet> submitAsync(final Traversal traversal) { - final byte[] serializedTraversal; - try { - serializedTraversal = Serializer.serializeObject(traversal); - } catch (Exception ex) { - throw new RuntimeException(ex); - } - - try { - return submitAsync(buildMessage(RequestMessage.build(Tokens.OPS_TRAVERSE) - .processor("traversal").addArg(Tokens.ARGS_GREMLIN, serializedTraversal)).create()); - } catch (Exception ex) { - throw new RuntimeException(ex); - } + return submitAsync(traversal.asAdmin().getBytecode()); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/688e3f55/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java ---------------------------------------------------------------------- diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java index 9d1a27d..4e276e0 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java @@ -47,19 +47,8 @@ public class DriverRemoteConnection implements RemoteConnection { public static final String GREMLIN_REMOTE_GRAPH_DRIVER_CLUSTERFILE = "gremlin.remoteGraph.driver.clusterFile"; - /** - * @deprecated As of release 3.2.2, replaced by {@link #GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME}. - */ - @Deprecated - public static final String GREMLIN_REMOTE_GRAPH_DRIVER_GRAPHNAME = "gremlin.remoteGraph.driver.graphName"; public static final String GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME = "gremlin.remoteGraph.driver.sourceName"; - - /** - * @deprecated As of release 3.2.2, replaced by {@link #GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME}. - */ - @Deprecated - private static final String DEFAULT_GRAPH = "graph"; private static final String DEFAULT_TRAVERSAL_SOURCE = "g"; private final Client client; @@ -73,10 +62,7 @@ public class DriverRemoteConnection implements RemoteConnection { if (conf.containsKey(GREMLIN_REMOTE_GRAPH_DRIVER_CLUSTERFILE) && conf.containsKey("clusterConfiguration")) throw new IllegalStateException(String.format("A configuration should not contain both '%s' and 'clusterConfiguration'", GREMLIN_REMOTE_GRAPH_DRIVER_CLUSTERFILE)); - if (conf.containsKey(GREMLIN_REMOTE_GRAPH_DRIVER_GRAPHNAME)) - connectionGraphName = conf.getString(GREMLIN_REMOTE_GRAPH_DRIVER_GRAPHNAME, DEFAULT_GRAPH); - else - connectionGraphName = conf.getString(GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME, DEFAULT_TRAVERSAL_SOURCE); + connectionGraphName = conf.getString(GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME, DEFAULT_TRAVERSAL_SOURCE); try { final Cluster cluster; @@ -105,10 +91,7 @@ public class DriverRemoteConnection implements RemoteConnection { * This constructor is largely just for unit testing purposes and should not typically be used externally. */ DriverRemoteConnection(final Cluster cluster, final Configuration conf) { - if (conf.containsKey(GREMLIN_REMOTE_GRAPH_DRIVER_GRAPHNAME)) - connectionGraphName = conf.getString(GREMLIN_REMOTE_GRAPH_DRIVER_GRAPHNAME, DEFAULT_GRAPH); - else - connectionGraphName = conf.getString(GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME, DEFAULT_TRAVERSAL_SOURCE); + connectionGraphName = conf.getString(GREMLIN_REMOTE_GRAPH_DRIVER_SOURCENAME, DEFAULT_TRAVERSAL_SOURCE); client = cluster.connect(Client.Settings.build().unrollTraversers(false).create()).alias(connectionGraphName); tryCloseCluster = false; @@ -121,7 +104,7 @@ public class DriverRemoteConnection implements RemoteConnection { * {@link RemoteConnection} to a graph on the server named "graph". */ public static DriverRemoteConnection using(final Cluster cluster) { - return using(cluster, "graph"); + return using(cluster, DEFAULT_TRAVERSAL_SOURCE); } /** @@ -138,7 +121,7 @@ public class DriverRemoteConnection implements RemoteConnection { * this method will bind the {@link RemoteConnection} to a graph on the server named "graph". */ public static DriverRemoteConnection using(final String clusterConfFile) { - return using(clusterConfFile, "graph"); + return using(clusterConfFile, DEFAULT_TRAVERSAL_SOURCE); } /** http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/688e3f55/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java index 5f0773e..2f0714a 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java @@ -28,10 +28,6 @@ import org.apache.tinkerpop.gremlin.groovy.engine.ScriptEngines; import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; -import org.apache.tinkerpop.gremlin.process.traversal.Traverser; -import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.HaltedTraverserStrategy; import org.apache.tinkerpop.gremlin.server.Context; import org.apache.tinkerpop.gremlin.server.GraphManager; import org.apache.tinkerpop.gremlin.server.GremlinServer; @@ -41,7 +37,6 @@ import org.apache.tinkerpop.gremlin.server.op.OpProcessorException; import org.apache.tinkerpop.gremlin.server.util.MetricManager; import org.apache.tinkerpop.gremlin.server.util.TraversalIterator; import org.apache.tinkerpop.gremlin.structure.Graph; -import org.apache.tinkerpop.gremlin.util.Serializer; import org.apache.tinkerpop.gremlin.util.function.ThrowingConsumer;; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,7 +45,6 @@ import javax.script.SimpleBindings; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeoutException; @@ -88,20 +82,8 @@ public class TraversalOpProcessor extends AbstractOpProcessor { final ThrowingConsumer<Context> op; switch (message.getOp()) { - case Tokens.OPS_TRAVERSE: - validateTraversalRequest(ctx, message); - - final Optional<Map<String, String>> traverseAliases = message.optionalArgs(Tokens.ARGS_ALIASES); - final Map.Entry<String, String> traverserKv = traverseAliases.get().entrySet().iterator().next(); - if (!ctx.getGraphManager().getGraphs().containsKey(traverserKv.getValue())) { - final String msg = String.format("The graph [%s] for alias [%s] is not configured on the server.", traverserKv.getValue(), traverserKv.getKey()); - throw new OpProcessorException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); - } - - op = this::iterateSerializedTraversal; - break; case Tokens.OPS_BYTECODE: - final Map<String, String> bytecodeAliases = validateTraversalRequest(ctx, message); + final Map<String, String> bytecodeAliases = validateTraversalRequest(message); final Map.Entry<String, String> bytecodeKv = bytecodeAliases.entrySet().iterator().next(); if (!ctx.getGraphManager().getTraversalSources().containsKey(bytecodeKv.getValue())) { final String msg = String.format("The traversal source [%s] for alias [%s] is not configured on the server.", bytecodeKv.getValue(), bytecodeKv.getKey()); @@ -121,7 +103,7 @@ public class TraversalOpProcessor extends AbstractOpProcessor { return op; } - private static Map<String,String> validateTraversalRequest(final Context ctx, final RequestMessage message) throws OpProcessorException { + private static Map<String,String> validateTraversalRequest(final RequestMessage message) throws OpProcessorException { if (!message.optionalArgs(Tokens.ARGS_GREMLIN).isPresent()) { final String msg = String.format("A message with an [%s] op code requires a [%s] argument.", Tokens.OPS_TRAVERSE, Tokens.ARGS_GREMLIN); throw new OpProcessorException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); @@ -233,107 +215,4 @@ public class TraversalOpProcessor extends AbstractOpProcessor { return metaData; } - - private void iterateSerializedTraversal(final Context context) throws OpProcessorException { - final RequestMessage msg = context.getRequestMessage(); - logger.debug("Traversal request {} for in thread {}", msg.getRequestId(), Thread.currentThread().getName()); - - final byte[] serializedTraversal = (byte[]) msg.getArgs().get(Tokens.ARGS_GREMLIN); - - // earlier validation in selection of this op method should free us to cast this without worry - final Map<String, String> aliases = (Map<String, String>) msg.optionalArgs(Tokens.ARGS_ALIASES).get(); - - final Traversal.Admin<?, ?> traversal; - try { - traversal = (Traversal.Admin) Serializer.deserializeObject(serializedTraversal); - } catch (Exception ex) { - throw new OpProcessorException("Could not deserialize the Traversal instance", - ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SERIALIZATION) - .statusMessage(ex.getMessage()).create()); - } - - if (traversal.isLocked()) - throw new OpProcessorException("Locked Traversals cannot be processed by the server", - ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SERIALIZATION) - .statusMessage("Locked Traversals cannot be processed by the server").create()); - - final Timer.Context timerContext = traversalOpTimer.time(); - try { - final ChannelHandlerContext ctx = context.getChannelHandlerContext(); - final GraphManager graphManager = context.getGraphManager(); - final String graphName = aliases.entrySet().iterator().next().getValue(); - final Graph graph = graphManager.getGraphs().get(graphName); - final boolean supportsTransactions = graph.features().graph().supportsTransactions(); - - configureTraversal(traversal, graph); - - context.getGremlinExecutor().getExecutorService().submit(() -> { - try { - if (supportsTransactions && graph.tx().isOpen()) graph.tx().rollback(); - - try { - // compile the traversal - without it getEndStep() has nothing in it - traversal.applyStrategies(); - handleIterator(context, new DetachingIterator<>(traversal)); - } catch (TimeoutException ex) { - final String errorMessage = String.format("Response iteration exceeded the configured threshold for request [%s] - %s", msg.getRequestId(), ex.getMessage()); - logger.warn(errorMessage); - ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT).statusMessage(errorMessage).create()); - if (supportsTransactions && graph.tx().isOpen()) graph.tx().rollback(); - return; - } catch (Exception ex) { - logger.warn(String.format("Exception processing a Traversal on iteration for request [%s].", msg.getRequestId()), ex); - ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR).statusMessage(ex.getMessage()).create()); - if (supportsTransactions && graph.tx().isOpen()) graph.tx().rollback(); - return; - } - - if (graph.features().graph().supportsTransactions()) graph.tx().commit(); - } catch (Exception ex) { - logger.warn(String.format("Exception processing a Traversal on request [%s].", msg.getRequestId()), ex); - ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR).statusMessage(ex.getMessage()).create()); - if (graph.features().graph().supportsTransactions() && graph.tx().isOpen()) graph.tx().rollback(); - } finally { - timerContext.stop(); - } - }); - - } catch (Exception ex) { - timerContext.stop(); - throw new OpProcessorException("Could not iterate the Traversal instance", - ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR).statusMessage(ex.getMessage()).create()); - } - } - - private static void configureTraversal(final Traversal.Admin<?, ?> traversal, final Graph graph) { - traversal.setGraph(graph); - final List<TraversalStrategy<?>> strategies = TraversalStrategies.GlobalCache.getStrategies(graph.getClass()).toList(); - final TraversalStrategy[] arrayOfStrategies = new TraversalStrategy[strategies.size()]; - strategies.toArray(arrayOfStrategies); - traversal.getStrategies().addStrategies(arrayOfStrategies); - } - - static class DetachingIterator<E> implements Iterator<Traverser.Admin<E>> { - - private Iterator<Traverser.Admin<E>> inner; - private HaltedTraverserStrategy haltedTraverserStrategy; - - public DetachingIterator(final Traversal.Admin<?, E> traversal) { - this.inner = traversal.getEndStep(); - this.haltedTraverserStrategy = traversal.getStrategies().getStrategy(HaltedTraverserStrategy.class).orElse( - Boolean.valueOf(System.getProperty("is.testing", "false")) ? - HaltedTraverserStrategy.detached() : - HaltedTraverserStrategy.reference()); - } - - @Override - public boolean hasNext() { - return this.inner.hasNext(); - } - - @Override - public Traverser.Admin<E> next() { - return this.haltedTraverserStrategy.halt(this.inner.next()); - } - } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/688e3f55/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinResultSetIntegrateTest.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinResultSetIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinResultSetIntegrateTest.java index 3a4ab27..956a87c 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinResultSetIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinResultSetIntegrateTest.java @@ -25,8 +25,8 @@ import org.apache.tinkerpop.gremlin.driver.Result; import org.apache.tinkerpop.gremlin.driver.ResultSet; import org.apache.tinkerpop.gremlin.driver.ser.GryoMessageSerializerV1d0; import org.apache.tinkerpop.gremlin.driver.ser.Serializers; +import org.apache.tinkerpop.gremlin.process.remote.traversal.step.util.BulkedResult; import org.apache.tinkerpop.gremlin.process.traversal.Path; -import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Graph; @@ -96,7 +96,7 @@ public class GremlinResultSetIntegrateTest extends AbstractGremlinServerIntegrat public void shouldHandleVertexResultFromTraversal() throws Exception { final Graph graph = TinkerGraph.open(); final GraphTraversalSource g = graph.traversal(); - final Client aliased = client.alias("graph"); + final Client aliased = client.alias("g"); final ResultSet resultSet = aliased.submit(g.V().both().both()); final List<Result> results = resultSet.all().get(); @@ -108,7 +108,7 @@ public class GremlinResultSetIntegrateTest extends AbstractGremlinServerIntegrat public void shouldHandleVertexResultFromTraversalAsTraversersUnrolled() throws Exception { final Graph graph = TinkerGraph.open(); final GraphTraversalSource g = graph.traversal(); - final Client aliased = client.alias("graph"); + final Client aliased = client.alias("g"); final ResultSet resultSetUnrolled = aliased.submit(g.V().both().barrier().both().barrier()); final List<Result> results = resultSetUnrolled.all().get(); @@ -121,11 +121,11 @@ public class GremlinResultSetIntegrateTest extends AbstractGremlinServerIntegrat final Graph graph = TinkerGraph.open(); final GraphTraversalSource g = graph.traversal(); final Client clientWithUnrolling = cluster.connect(Client.Settings.build().unrollTraversers(false).create()); - final Client aliased = clientWithUnrolling.alias("graph"); + final Client aliased = clientWithUnrolling.alias("g"); final ResultSet resultSet = aliased.submit(g.V().both().barrier().both().barrier()); final List<Result> results = resultSet.all().get(); - assertThat(results.get(0).getObject(), CoreMatchers.instanceOf(Traverser.class)); + assertThat(results.get(0).getObject(), CoreMatchers.instanceOf(BulkedResult.class)); assertEquals(6, results.size()); aliased.close();
