Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-1458 575ff2774 -> be8b2e22e
fixed logic in DriverRemoteSideEffects, don't clear local side effect cache Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/be8b2e22 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/be8b2e22 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/be8b2e22 Branch: refs/heads/TINKERPOP-1458 Commit: be8b2e22e715b0f9081cde98e20b3c0d30d7c0f3 Parents: 575ff27 Author: davebshow <[email protected]> Authored: Thu Sep 29 13:04:57 2016 -0400 Committer: davebshow <[email protected]> Committed: Thu Sep 29 13:04:57 2016 -0400 ---------------------------------------------------------------------- .../DriverRemoteTraversalSideEffects.java | 48 ++++++++++---------- .../server/GremlinServerIntegrateTest.java | 13 +++--- 2 files changed, 31 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/be8b2e22/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 c565dfa..d2fced5 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 @@ -39,7 +39,7 @@ import java.util.stream.Collectors; public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSideEffects { private final Client client; - private Set<String> keys = null; + private Set<String> keys = Collections.emptySet(); private final UUID serverSideEffect; private final Host host; @@ -56,22 +56,27 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSid @Override public <V> V get(final String key) throws IllegalArgumentException { if (!sideEffects.containsKey(key)) { - // specify the ARGS_HOST so that the LoadBalancingStrategy is subverted and the connection is forced - // from the specified host (i.e. the host from the previous request as that host will hold the side-effects) - final RequestMessage msg = RequestMessage.build(Tokens.OPS_GATHER) - .addArg(Tokens.ARGS_SIDE_EFFECT, serverSideEffect) - .addArg(Tokens.ARGS_SIDE_EFFECT_KEY, key) - .addArg(Tokens.ARGS_HOST, host) - .processor("traversal").create(); - try { - final Result result = client.submitAsync(msg).get().one(); - sideEffects.put(key, null == result ? null : result.getObject()); - } catch (Exception ex) { - final Throwable root = ExceptionUtils.getRootCause(ex); - if (root.getMessage().equals("Could not find side-effects for " + serverSideEffect + ".")) - sideEffects.put(key, null); - else - throw new RuntimeException("Could not get keys", root); + if (!closed) { + // specify the ARGS_HOST so that the LoadBalancingStrategy is subverted and the connection is forced + // from the specified host (i.e. the host from the previous request as that host will hold the side-effects) + final RequestMessage msg = RequestMessage.build(Tokens.OPS_GATHER) + .addArg(Tokens.ARGS_SIDE_EFFECT, serverSideEffect) + .addArg(Tokens.ARGS_SIDE_EFFECT_KEY, key) + .addArg(Tokens.ARGS_HOST, host) + .processor("traversal").create(); + try { + final Result result = client.submitAsync(msg).get().one(); + sideEffects.put(key, null == result ? null : result.getObject()); + keys.add(key); + } catch (Exception ex) { + final Throwable root = ExceptionUtils.getRootCause(ex); + if (root.getMessage().equals("Could not find side-effects for " + serverSideEffect + ".")) + sideEffects.put(key, null); + else + throw new RuntimeException("Could not get keys", root); + } + } else { + sideEffects.put(key, null); } } @@ -80,7 +85,7 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSid @Override public Set<String> keys() { - if (null == keys) { + if (!closed) { // specify the ARGS_HOST so that the LoadBalancingStrategy is subverted and the connection is forced // from the specified host (i.e. the host from the previous request as that host will hold the side-effects) final RequestMessage msg = RequestMessage.build(Tokens.OPS_KEYS) @@ -91,9 +96,7 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSid keys = client.submitAsync(msg).get().all().get().stream().map(r -> r.getString()).collect(Collectors.toSet()); } catch (Exception ex) { final Throwable root = ExceptionUtils.getRootCause(ex); - if (root.getMessage().equals("Could not find side-effects for " + serverSideEffect + ".")) - keys = Collections.emptySet(); - else + if (!root.getMessage().equals("Could not find side-effects for " + serverSideEffect + ".")) throw new RuntimeException("Could not get keys", root); } } @@ -101,7 +104,6 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSid return keys; } - @Override public void close() throws Exception { if (!closed) { @@ -111,8 +113,6 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSid .processor("traversal").create(); try { client.submitAsync(msg).get(); - sideEffects.clear(); - keys = null; closed = true; } catch (Exception ex) { final Throwable root = ExceptionUtils.getRootCause(ex); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/be8b2e22/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java index a1d019b..e6e0021 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java @@ -70,6 +70,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -90,6 +91,7 @@ import static org.hamcrest.core.StringEndsWith.endsWith; import static org.hamcrest.core.StringStartsWith.startsWith; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; import static org.junit.Assert.assertEquals; @@ -831,17 +833,16 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration } @Test - public void shouldCloseSideEffects() throws Exception { + public void shouldCloseLocalSideEffects() throws Exception { final Graph graph = EmptyGraph.instance(); final GraphTraversalSource g = graph.traversal().withRemote(conf); g.addV("person").property("age", 20).iterate(); g.addV("person").property("age", 10).iterate(); - final GraphTraversal traversal = g.V().aggregate("a"); + final GraphTraversal traversal = g.V().aggregate("a").aggregate("b"); traversal.iterate(); - final Set sideEffects = traversal.asAdmin().getSideEffects().keys(); - assertTrue(sideEffects.contains("a")); + final List sideEffects = traversal.asAdmin().getSideEffects().get("a"); + assertFalse(sideEffects.isEmpty()); traversal.asAdmin().getSideEffects().close(); - final Set emptySideEffects = traversal.asAdmin().getSideEffects().keys(); - assertTrue(emptySideEffects.isEmpty()); + assertNull(traversal.asAdmin().getSideEffects().get("b")); } }
