You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by da...@apache.org on 2016/09/29 17:05:09 UTC

tinkerpop git commit: fixed logic in DriverRemoteSideEffects, don't clear local side effect cache

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 <da...@gmail.com>
Authored: Thu Sep 29 13:04:57 2016 -0400
Committer: davebshow <da...@gmail.com>
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"));
     }
 }