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 2015/03/18 13:09:15 UTC
incubator-tinkerpop git commit: Changed PartitionStrategy to not
inject a has after addV/E steps.
Repository: incubator-tinkerpop
Updated Branches:
refs/heads/master 005dd6d02 -> 36424c8b5
Changed PartitionStrategy to not inject a has after addV/E steps.
If you add a has after those steps then it is not possible to write to a partition that you aren't also reading from. That was possible with TP2 - seems worth keeping that way.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/36424c8b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/36424c8b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/36424c8b
Branch: refs/heads/master
Commit: 36424c8b5beb0b11c1f42e06adca244393fe1638
Parents: 005dd6d
Author: Stephen Mallette <sp...@apache.org>
Authored: Wed Mar 18 08:05:27 2015 -0400
Committer: Stephen Mallette <sp...@apache.org>
Committed: Wed Mar 18 08:05:27 2015 -0400
----------------------------------------------------------------------
.../strategy/decoration/PartitionStrategy.java | 32 ++-
.../decoration/PartitionStrategyTest.java | 243 ++++++++++---------
.../PartitionStrategyProcessTest.java | 2 +-
3 files changed, 157 insertions(+), 120 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/36424c8b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
index 561550b..c634a3a 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
@@ -33,27 +33,38 @@ public class PartitionStrategy extends AbstractTraversalStrategy {
public PartitionStrategy(final String partitionKey, final String partition) {
this.writePartition = partition;
- this.addReadPartition(partition);
this.partitionKey = partitionKey;
+ readPartitions.add(writePartition);
}
public String getWritePartition() {
return this.writePartition;
}
+ /**
+ * Set the partition to be used for future writes. The write partition is always included in the list of
+ * current read partitions.
+ */
public void setWritePartition(final String writePartition) {
this.writePartition = writePartition;
- this.readPartitions.add(writePartition);
}
public String getPartitionKey() {
return this.partitionKey;
}
+ /**
+ * Gets the set of read partitions which will include the current write partition even if not explicitly added
+ * as a read partition otherwise.
+ */
public Set<String> getReadPartitions() {
- return Collections.unmodifiableSet(this.readPartitions);
+ return Collections.unmodifiableSet(readPartitions);
}
+ /**
+ * Removes the specified read partition. Note that the current write partition cannot be removed as a read
+ * partition.
+ */
public void removeReadPartition(final String readPartition) {
this.readPartitions.remove(readPartition);
}
@@ -62,22 +73,27 @@ public class PartitionStrategy extends AbstractTraversalStrategy {
this.readPartitions.add(readPartition);
}
+ /**
+ * Clears all of the read partitions. Note that the current write partition cannot be cleared as a read
+ * partition.
+ */
public void clearReadPartitions() {
this.readPartitions.clear();
}
@Override
public void apply(final Traversal.Admin<?, ?> traversal) {
- // todo: what about "add vertex"
+ final Set<String> readPartitionsWithWritePartition = new HashSet<>(readPartitions);
+ readPartitionsWithWritePartition.add(writePartition);
+
+ // no need to add has after mutating steps because we want to make it so that the write partition can
+ // be independent of the read partition. in other words, i don't need to be able to read from a partition
+ // in order to write to it.
final List<Step> stepsToInsertHasAfter = new ArrayList<>();
stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(GraphStep.class, traversal));
stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(VertexStep.class, traversal));
stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(EdgeOtherVertexStep.class, traversal));
stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(EdgeVertexStep.class, traversal));
- stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(AddEdgeStep.class, traversal));
- stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(AddEdgeByPathStep.class, traversal));
- stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(AddVertexStep.class, traversal));
- stepsToInsertHasAfter.addAll(TraversalHelper.getStepsOfAssignableClass(AddVertexStartStep.class, traversal));
// all steps that return a vertex need to have has(paritionKey,within,partitionValues) injected after it
stepsToInsertHasAfter.forEach(s -> TraversalHelper.insertAfterStep(
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/36424c8b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyTest.java
index efdcfe3..ec34360 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyTest.java
@@ -18,6 +18,7 @@ import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.util.ElementHelper;
import org.javatuples.Pair;
import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -33,125 +34,145 @@ import static org.mockito.Mockito.mock;
/**
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
-@RunWith(Parameterized.class)
+@RunWith(Enclosed.class)
public class PartitionStrategyTest {
- private static Traversal traversalWithAddV;
- static {
- final Graph mockedGraph = mock(Graph.class);
- final DefaultGraphTraversal t = new DefaultGraphTraversal<>(mockedGraph);
- t.asAdmin().addStep(new GraphStep<>(t.asAdmin(), Vertex.class));
- traversalWithAddV = t.addV();
- }
-
- @Parameterized.Parameters(name = "{0}")
- public static Iterable<Object[]> data() {
- return Arrays.asList(new Object[][]{
- {"bothV()", __.bothV(), 1, false},
- {"inV()", __.inV(), 1, false},
- {"outV()", __.outV(), 1, false},
- {"in()", __.in(), 1, false},
- {"in(args)", __.in("test"), 1, false},
- {"both()", __.both(), 1, false},
- {"both(args)", __.both("test"), 1, false},
- {"out()", __.out(), 1, false},
- {"out(args)", __.out("test"), 1, false},
- {"out().inE().otherV", __.out().inE().otherV(), 3, false},
- {"addV()", traversalWithAddV, 2, true},
- {"addInE()", __.addInE("test", "x"), 1, true},
- {"addOutE()", __.addOutE("test", "x"), 1, true},
- {"addInE()", __.addInE("test", "x", "other", "args"), 1, true},
- {"addOutE()", __.addOutE("test", "x", "other", "args"), 1, true},
- {"addBothE(OUT)", __.addE(Direction.OUT, "test", "x"), 1, true},
- {"addBothE(IN)", __.addE(Direction.IN, "test", "x"), 1, true},
- {"in().out()", __.in().out(), 2, false},
- {"out().out().out()", __.out().out().out(), 3, false},
- {"in().out().in()", __.in().out().in(), 3, false},
- {"inE().outV().inE().outV()", __.inE().outV().inE().outV(), 4, false}});
- }
-
- @Parameterized.Parameter(value = 0)
- public String name;
-
- @Parameterized.Parameter(value = 1)
- public Traversal traversal;
- @Parameterized.Parameter(value = 2)
- public int expectedInsertedSteps;
-
- @Parameterized.Parameter(value = 3)
- public boolean hasMutatingStep;
-
- @Test
- public void shouldIncludeAdditionalHasStepsAndAppendPartitionOnMutatingSteps() {
- final PartitionStrategy strategy = new PartitionStrategy("p", "a");
+ public static class PartitionKeyBehavior {
+ @Test
+ public void shouldMakeTheInitialWritePartitionPartOfTheReadPartition() {
+ final PartitionStrategy strategy = new PartitionStrategy("p", "a");
+ assertEquals("a", strategy.getReadPartitions().iterator().next());
+ assertEquals(1, strategy.getReadPartitions().size());
+ }
- if (hasMutatingStep) {
- if (TraversalHelper.hasStepOfAssignableClass(AddEdgeStep.class, traversal.asAdmin())) {
- final Direction d = TraversalHelper.getStepsOfClass(AddEdgeStep.class, traversal.asAdmin()).get(0).getDirection();
- strategy.apply(traversal.asAdmin());
+ @Test
+ public void shouldSetTheWritePartitionOnConstruction() {
+ final PartitionStrategy strategy = new PartitionStrategy("p", "a");
+ assertEquals("p", strategy.getPartitionKey());
+ }
+ }
- final List<AddEdgeStep> addEdgeSteps = TraversalHelper.getStepsOfAssignableClass(AddEdgeStep.class, traversal.asAdmin());
- assertEquals(1, addEdgeSteps.size());
-
- addEdgeSteps.forEach(s -> {
- final Object[] keyValues = s.getKeyValues();
- final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
- assertEquals("test", s.getEdgeLabel());
- assertEquals(d, s.getDirection());
- assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
- });
- } else if (TraversalHelper.hasStepOfAssignableClass(AddEdgeByPathStep.class, traversal.asAdmin())) {
- final Direction d = TraversalHelper.getStepsOfClass(AddEdgeByPathStep.class, traversal.asAdmin()).get(0).getDirection();
- strategy.apply(traversal.asAdmin());
+ @RunWith(Parameterized.class)
+ public static class TraverseBehavior {
+ private static Traversal traversalWithAddV;
- final List<AddEdgeByPathStep> addEdgeSteps = TraversalHelper.getStepsOfAssignableClass(AddEdgeByPathStep.class, traversal.asAdmin());
- assertEquals(1, addEdgeSteps.size());
-
- addEdgeSteps.forEach(s -> {
- final Object[] keyValues = s.getKeyValues();
- final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
- assertEquals("test", s.getEdgeLabel());
- assertEquals(d, s.getDirection());
- assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
- });
- } else if (TraversalHelper.hasStepOfAssignableClass(AddVertexStep.class, traversal.asAdmin())) {
- strategy.apply(traversal.asAdmin());
+ static {
+ final Graph mockedGraph = mock(Graph.class);
+ final DefaultGraphTraversal t = new DefaultGraphTraversal<>(mockedGraph);
+ t.asAdmin().addStep(new GraphStep<>(t.asAdmin(), Vertex.class));
+ traversalWithAddV = t.addV();
+ }
- final List<AddVertexStep> addVertexSteps = TraversalHelper.getStepsOfAssignableClass(AddVertexStep.class, traversal.asAdmin());
- assertEquals(1, addVertexSteps.size());
+ @Parameterized.Parameters(name = "{0}")
+ public static Iterable<Object[]> data() {
+ return Arrays.asList(new Object[][]{
+ {"bothV()", __.bothV(), 1, false},
+ {"inV()", __.inV(), 1, false},
+ {"outV()", __.outV(), 1, false},
+ {"in()", __.in(), 1, false},
+ {"in(args)", __.in("test"), 1, false},
+ {"both()", __.both(), 1, false},
+ {"both(args)", __.both("test"), 1, false},
+ {"out()", __.out(), 1, false},
+ {"out(args)", __.out("test"), 1, false},
+ {"out().inE().otherV", __.out().inE().otherV(), 3, false},
+ {"addV()", traversalWithAddV, 1, true},
+ {"addInE()", __.addInE("test", "x"), 0, true},
+ {"addOutE()", __.addOutE("test", "x"), 0, true},
+ {"addInE()", __.addInE("test", "x", "other", "args"), 0, true},
+ {"addOutE()", __.addOutE("test", "x", "other", "args"), 0, true},
+ {"addBothE(OUT)", __.addE(Direction.OUT, "test", "x"), 0, true},
+ {"addBothE(IN)", __.addE(Direction.IN, "test", "x"), 0, true},
+ {"in().out()", __.in().out(), 2, false},
+ {"out().out().out()", __.out().out().out(), 3, false},
+ {"in().out().in()", __.in().out().in(), 3, false},
+ {"inE().outV().inE().outV()", __.inE().outV().inE().outV(), 4, false}});
+ }
- addVertexSteps.forEach(s -> {
- final Object[] keyValues = s.getKeyValues();
- final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
- assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
- });
- } else if (TraversalHelper.hasStepOfAssignableClass(AddVertexStartStep.class, traversal.asAdmin())) {
+ @Parameterized.Parameter(value = 0)
+ public String name;
+
+ @Parameterized.Parameter(value = 1)
+ public Traversal traversal;
+
+ @Parameterized.Parameter(value = 2)
+ public int expectedInsertedSteps;
+
+ @Parameterized.Parameter(value = 3)
+ public boolean hasMutatingStep;
+
+ @Test
+ public void shouldIncludeAdditionalHasStepsAndAppendPartitionOnMutatingSteps() {
+ final PartitionStrategy strategy = new PartitionStrategy("p", "a");
+
+ if (hasMutatingStep) {
+ if (TraversalHelper.hasStepOfAssignableClass(AddEdgeStep.class, traversal.asAdmin())) {
+ final Direction d = TraversalHelper.getStepsOfClass(AddEdgeStep.class, traversal.asAdmin()).get(0).getDirection();
+ strategy.apply(traversal.asAdmin());
+
+ final List<AddEdgeStep> addEdgeSteps = TraversalHelper.getStepsOfAssignableClass(AddEdgeStep.class, traversal.asAdmin());
+ assertEquals(1, addEdgeSteps.size());
+
+ addEdgeSteps.forEach(s -> {
+ final Object[] keyValues = s.getKeyValues();
+ final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
+ assertEquals("test", s.getEdgeLabel());
+ assertEquals(d, s.getDirection());
+ assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
+ });
+ } else if (TraversalHelper.hasStepOfAssignableClass(AddEdgeByPathStep.class, traversal.asAdmin())) {
+ final Direction d = TraversalHelper.getStepsOfClass(AddEdgeByPathStep.class, traversal.asAdmin()).get(0).getDirection();
+ strategy.apply(traversal.asAdmin());
+
+ final List<AddEdgeByPathStep> addEdgeSteps = TraversalHelper.getStepsOfAssignableClass(AddEdgeByPathStep.class, traversal.asAdmin());
+ assertEquals(1, addEdgeSteps.size());
+
+ addEdgeSteps.forEach(s -> {
+ final Object[] keyValues = s.getKeyValues();
+ final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
+ assertEquals("test", s.getEdgeLabel());
+ assertEquals(d, s.getDirection());
+ assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
+ });
+ } else if (TraversalHelper.hasStepOfAssignableClass(AddVertexStep.class, traversal.asAdmin())) {
+ strategy.apply(traversal.asAdmin());
+
+ final List<AddVertexStep> addVertexSteps = TraversalHelper.getStepsOfAssignableClass(AddVertexStep.class, traversal.asAdmin());
+ assertEquals(1, addVertexSteps.size());
+
+ addVertexSteps.forEach(s -> {
+ final Object[] keyValues = s.getKeyValues();
+ final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
+ assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
+ });
+ } else if (TraversalHelper.hasStepOfAssignableClass(AddVertexStartStep.class, traversal.asAdmin())) {
+ strategy.apply(traversal.asAdmin());
+
+ final List<AddVertexStartStep> addVertexSteps = TraversalHelper.getStepsOfAssignableClass(AddVertexStartStep.class, traversal.asAdmin());
+ assertEquals(1, addVertexSteps.size());
+
+ addVertexSteps.forEach(s -> {
+ final Object[] keyValues = s.getKeyValues();
+ final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
+ assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
+ });
+ } else
+ fail("This test should not be marked as having a mutating step or there is something else amiss.");
+ } else {
strategy.apply(traversal.asAdmin());
-
- final List<AddVertexStartStep> addVertexSteps = TraversalHelper.getStepsOfAssignableClass(AddVertexStartStep.class, traversal.asAdmin());
- assertEquals(1, addVertexSteps.size());
-
- addVertexSteps.forEach(s -> {
- final Object[] keyValues = s.getKeyValues();
- final List<Pair<String, Object>> pairs = ElementHelper.asPairs(keyValues);
- assertTrue(pairs.stream().anyMatch(p -> p.getValue0().equals("p") && p.getValue1().equals("a")));
- });
- } else
- fail("This test should not be marked as having a mutating step or there is something else amiss.");
- } else {
- strategy.apply(traversal.asAdmin());
+ }
+
+ final List<HasStep> steps = TraversalHelper.getStepsOfClass(HasStep.class, traversal.asAdmin());
+ assertEquals(expectedInsertedSteps, steps.size());
+
+ final List<String> keySet = new ArrayList<>(strategy.getReadPartitions());
+ steps.forEach(s -> {
+ assertEquals(1, s.getHasContainers().size());
+ final HasContainer hasContainer = (HasContainer) s.getHasContainers().get(0);
+ assertEquals("p", hasContainer.key);
+ assertEquals(keySet, hasContainer.value);
+ assertEquals(Contains.within, hasContainer.predicate);
+ });
}
-
- final List<HasStep> steps = TraversalHelper.getStepsOfClass(HasStep.class, traversal.asAdmin());
- assertEquals(expectedInsertedSteps, steps.size());
-
- final List<String> keySet = new ArrayList<>(strategy.getReadPartitions());
- steps.forEach(s -> {
- assertEquals(1, s.getHasContainers().size());
- final HasContainer hasContainer = (HasContainer) s.getHasContainers().get(0);
- assertEquals("p", hasContainer.key);
- assertEquals(keySet, hasContainer.value);
- assertEquals(Contains.within, hasContainer.predicate);
- });
}
}
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/36424c8b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyProcessTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyProcessTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyProcessTest.java
index d4a8a61..1b3de16 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyProcessTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategyProcessTest.java
@@ -148,9 +148,9 @@ public class PartitionStrategyProcessTest extends AbstractGremlinProcessTest {
partitionStrategy.setWritePartition("B");
final Vertex vB = source.addV("any", "b").next();
- boolean x = source.V(vA.id()).hasNext();
source.V(vA.id()).addOutE("a->b", vB).next();
+ partitionStrategy.addReadPartition("B");
partitionStrategy.setWritePartition("C");
final Vertex vC = source.addV("any", "c").next();
final Edge eBtovC = source.V(vB.id()).addOutE("b->c", vC).next();