Fixed up TraversalOpProcessor to use HaltedTraversalStrategy to decide detachement with ReferenceFactory being the default (as before). This allowed me to open uup a bunch more OPT_OUT tests on RemoteGraph.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/408755de Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/408755de Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/408755de Branch: refs/heads/TINKERPOP-1254 Commit: 408755de33de1eb23441347ac6bbe0b084828d9c Parents: 2eafeef Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Tue Jun 21 11:43:14 2016 -0600 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Tue Jun 21 12:06:24 2016 -0600 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../gremlin/process/remote/RemoteGraph.java | 20 ++++++++----------- .../decoration/HaltedTraverserStrategy.java | 8 +++++--- .../driver/remote/DriverRemoteConnection.java | 3 ++- .../op/traversal/TraversalOpProcessor.java | 21 ++++++++++++-------- 5 files changed, 29 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/408755de/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ea940a8..1ba08d4 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/incubator-tinkerpop/master/docs/ TinkerPop 3.2.1 (NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* `TraversalOpProcessor` (`RemoteConnection`) uses `HaltedTraverserStrategy` metadata to determine detachment procedure prior to returning results. * Allow DFS paths in `HADOOP_GREMLIN_LIBS`. * Added a safer serializer infrastructure for use with `SparkGraphComputer` that uses `KryoSerializer` and the new `GryoRegistrator`. * Added `HaltedTraverserStrategy` to allow users to get back different element detachments in OLAP. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/408755de/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/RemoteGraph.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/RemoteGraph.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/RemoteGraph.java index f46fe90..590398e 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/RemoteGraph.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/RemoteGraph.java @@ -46,6 +46,10 @@ import java.util.Iterator; @Graph.OptIn(Graph.OptIn.SUITE_PROCESS_STANDARD) @Graph.OptIn(Graph.OptIn.SUITE_PROCESS_COMPUTER) @Graph.OptOut( + test = "org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTest$Traversals", + method = "g_withSideEffectXa_g_VX2XX_VX1X_out_whereXneqXaXX", + reason = "RemoteGraph can not handle a sideEffect that is spawned by a traversal") +@Graph.OptOut( test = "org.apache.tinkerpop.gremlin.process.traversal.CoreTraversalTest", method = "shouldNeverPropagateANoBulkTraverser", reason = "RemoteGraph can't serialize a lambda so the test fails before it has a chance for the Traversal to be evaluated") @@ -54,6 +58,10 @@ import java.util.Iterator; method = "shouldNeverPropagateANullValuedTraverser", reason = "RemoteGraph can't serialize a lambda so the test fails before it has a chance for the Traversal to be evaluated") @Graph.OptOut( + test = "org.apache.tinkerpop.gremlin.process.traversal.step.map.ProgramTest", + method = "*", + reason = "RemoteGraph retrieves detached vertices that can't be attached to a remote OLAP graph") +@Graph.OptOut( test = "org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.ElementIdStrategyProcessTest", method = "*", reason = "RemoteGraph does not support ElementIdStrategy at this time - it requires a lambda in construction which is not serializable") @@ -66,18 +74,6 @@ import java.util.Iterator; method = "*", reason = "RemoteGraph does not support PartitionStrategy at this time") @Graph.OptOut( - test = "org.apache.tinkerpop.gremlin.process.traversal.step.map.PeerPressureTest", - method = "g_V_peerPressure", - reason = "RemoteGraph retrieves detached vertices that can't be attached to a remote OLAP graph") -@Graph.OptOut( - test = "org.apache.tinkerpop.gremlin.process.traversal.step.map.PageRankTest", - method = "g_V_pageRank", - reason = "RemoteGraph retrieves detached vertices that can't be attached to a remote OLAP graph") -@Graph.OptOut( - test = "org.apache.tinkerpop.gremlin.process.traversal.step.map.ProgramTest", - method = "g_V_programXpageRankX", - reason = "RemoteGraph retrieves detached vertices that can't be attached to a remote OLAP graph") -@Graph.OptOut( test = "org.apache.tinkerpop.gremlin.process.computer.ranking.pagerank.PageRankVertexProgramTest", method = "*", reason = "RemoteGraph does not support direct Graph.compute() access") http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/408755de/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/HaltedTraverserStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/HaltedTraverserStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/HaltedTraverserStrategy.java index 45966f0..1a2c207 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/HaltedTraverserStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/HaltedTraverserStrategy.java @@ -32,11 +32,13 @@ import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceFactory; public final class HaltedTraverserStrategy extends AbstractTraversalStrategy<TraversalStrategy.DecorationStrategy> implements TraversalStrategy.DecorationStrategy { private final Class haltedTraverserFactory; + private final boolean useReference; private HaltedTraverserStrategy(final Class haltedTraverserFactory) { - if (haltedTraverserFactory.equals(DetachedFactory.class) || haltedTraverserFactory.equals(ReferenceFactory.class)) + if (haltedTraverserFactory.equals(DetachedFactory.class) || haltedTraverserFactory.equals(ReferenceFactory.class)) { this.haltedTraverserFactory = haltedTraverserFactory; - else + this.useReference = ReferenceFactory.class.equals(this.haltedTraverserFactory); + } else throw new IllegalArgumentException("The provided traverser detachment factory is unknown: " + haltedTraverserFactory); } @@ -49,7 +51,7 @@ public final class HaltedTraverserStrategy extends AbstractTraversalStrategy<Tra } public <R> Traverser.Admin<R> halt(final Traverser.Admin<R> traverser) { - if (ReferenceFactory.class.equals(this.haltedTraverserFactory)) + if (this.useReference) traverser.set(ReferenceFactory.detach(traverser.get())); else traverser.set(DetachedFactory.detach(traverser.get(), true)); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/408755de/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 a6303a6..2bea54b 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 @@ -22,6 +22,7 @@ import org.apache.commons.configuration.Configuration; import org.apache.tinkerpop.gremlin.driver.Client; import org.apache.tinkerpop.gremlin.driver.Cluster; import org.apache.tinkerpop.gremlin.driver.Result; +import org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.decoration.VertexProgramStrategy; import org.apache.tinkerpop.gremlin.process.remote.RemoteConnection; import org.apache.tinkerpop.gremlin.process.remote.RemoteConnectionException; import org.apache.tinkerpop.gremlin.process.remote.RemoteGraph; @@ -160,7 +161,7 @@ public class DriverRemoteConnection implements RemoteConnection { @Override public <E> Iterator<Traverser.Admin<E>> submit(final Traversal<?, E> t) throws RemoteConnectionException { try { - if (attachElements) { + if (attachElements && !t.asAdmin().getStrategies().toList().stream().filter(s -> s instanceof VertexProgramStrategy).findAny().isPresent()) { if (!conf.isPresent()) throw new IllegalStateException("Traverser can't be reattached for testing"); final Graph graph = ((Supplier<Graph>) conf.get().getProperty("hidden.for.testing.only")).get(); return new AttachingTraverserIterator<>(client.submit(t).iterator(), graph); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/408755de/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 bdb692d..8bff220 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,6 +28,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal; 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; @@ -45,7 +46,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.Future; import java.util.concurrent.TimeoutException; import static com.codahale.metrics.MetricRegistry.name; @@ -127,7 +127,7 @@ public class TraversalOpProcessor extends AbstractOpProcessor { // 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; + final Traversal.Admin<?, ?> traversal; try { traversal = (Traversal.Admin) Serializer.deserializeObject(serializedTraversal); } catch (Exception ex) { @@ -158,7 +158,7 @@ public class TraversalOpProcessor extends AbstractOpProcessor { try { // compile the traversal - without it getEndStep() has nothing in it traversal.applyStrategies(); - handleIterator(context, new DetachingIterator<>(traversal.getEndStep())); + 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); @@ -200,19 +200,24 @@ public class TraversalOpProcessor extends AbstractOpProcessor { static class DetachingIterator<E> implements Iterator<Traverser.Admin<E>> { private Iterator<Traverser.Admin<E>> inner; - - public DetachingIterator(final Iterator<Traverser.Admin<E>> toDetach) { - inner = toDetach; + private HaltedTraverserStrategy haltedTraverserStrategy; + + public DetachingIterator(final Traversal.Admin<?, E> traversal) { + this.inner = traversal.getEndStep(); + this.haltedTraverserStrategy = (HaltedTraverserStrategy) traversal.getStrategies().toList().stream().filter(s -> s instanceof HaltedTraverserStrategy).findAny().orElse( + Boolean.valueOf(System.getProperty("is.testing", "false")) ? + HaltedTraverserStrategy.detached() : + HaltedTraverserStrategy.reference()); } @Override public boolean hasNext() { - return inner.hasNext(); + return this.inner.hasNext(); } @Override public Traverser.Admin<E> next() { - return inner.next().detach(); + return this.haltedTraverserStrategy.halt(this.inner.next()); } } }
