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/02/17 19:41:57 UTC
incubator-tinkerpop git commit: Remove Thread interruption in
Traversals.
Repository: incubator-tinkerpop
Updated Branches:
refs/heads/master aeb8ad28c -> 575048d0d
Remove Thread interruption in Traversals.
Doesn't work cleanly for all graphs and introduces some complexity that might be otherwise handled in a less intrusive fashion.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/575048d0
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/575048d0
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/575048d0
Branch: refs/heads/master
Commit: 575048d0dda94de8f417dd368533bf352551308d
Parents: aeb8ad2
Author: Stephen Mallette <sp...@apache.org>
Authored: Tue Feb 17 13:38:59 2015 -0500
Committer: Stephen Mallette <sp...@apache.org>
Committed: Tue Feb 17 13:38:59 2015 -0500
----------------------------------------------------------------------
.../tinkerpop/gremlin/process/Traversal.java | 6 --
.../process/TraversalInterruptedException.java | 50 ----------
.../groovy/engine/GremlinExecutorTest.java | 67 +------------
.../jsr223/GremlinGroovyScriptEngineTest.java | 30 ------
.../gremlin/groovy/engine/GremlinExecutor.java | 4 +-
.../process/traversal/CoreTraversalTest.java | 98 --------------------
.../structure/hdfs/HadoopElementIterator.java | 6 --
7 files changed, 3 insertions(+), 258 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/Traversal.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/Traversal.java
index 94139ea..92ef8ec 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/Traversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/Traversal.java
@@ -29,7 +29,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
import org.apache.tinkerpop.gremlin.process.traverser.TraverserRequirement;
import org.apache.tinkerpop.gremlin.process.util.BulkSet;
import org.apache.tinkerpop.gremlin.structure.Graph;
-import org.apache.tinkerpop.gremlin.util.InterruptedRuntimeException;
import java.io.Serializable;
import java.util.ArrayList;
@@ -91,7 +90,6 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
final List<E> result = new ArrayList<>();
int counter = 0;
while (counter++ < amount && this.hasNext()) {
- if (Thread.interrupted()) throw new TraversalInterruptedException(this);
result.add(this.next());
}
return result;
@@ -137,7 +135,6 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
// use the end step so the results are bulked
final Step<?, E> endStep = this.asAdmin().getEndStep();
while (true) {
- if (Thread.interrupted()) throw new TraversalInterruptedException(this);
final Traverser<E> traverser = endStep.next();
TraversalHelper.addToCollection(collection, traverser.get(), traverser.bulk());
}
@@ -159,7 +156,6 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
// use the end step so the results are bulked
final Step<?, E> endStep = this.asAdmin().getEndStep();
while (true) {
- if (Thread.interrupted()) throw new TraversalInterruptedException(this);
endStep.next();
}
} catch (final NoSuchElementException ignored) {
@@ -178,7 +174,6 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
public default <E2> void forEachRemaining(final Class<E2> endType, final Consumer<E2> consumer) {
try {
while (true) {
- if (Thread.interrupted()) throw new TraversalInterruptedException(this);
consumer.accept((E2) next());
}
} catch (final NoSuchElementException ignore) {
@@ -190,7 +185,6 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable {
public default void forEachRemaining(final Consumer<? super E> action) {
try {
while (true) {
- if (Thread.interrupted()) throw new TraversalInterruptedException(this);
action.accept(next());
}
} catch (final NoSuchElementException ignore) {
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/TraversalInterruptedException.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/TraversalInterruptedException.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/TraversalInterruptedException.java
deleted file mode 100644
index dc1f803..0000000
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/TraversalInterruptedException.java
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.tinkerpop.gremlin.process;
-
-import org.apache.tinkerpop.gremlin.util.InterruptedRuntimeException;
-
-import java.util.Optional;
-
-/**
- * An unchecked exception thrown when the current thread processing a {@link Traversal} is interrupted.
- *
- * @author Stephen Mallette (http://stephen.genoprime.com)
- */
-public class TraversalInterruptedException extends InterruptedRuntimeException {
- private final Optional<Traversal> interruptedTraversal;
-
- public TraversalInterruptedException(final Throwable t) {
- this(null, t);
- }
-
- public TraversalInterruptedException(final Traversal interruptedTraversal) {
- this(interruptedTraversal, null);
- }
-
- public TraversalInterruptedException(final Traversal interruptedTraversal, final Throwable cause) {
- super(String.format("The %s thread received interruption notification while iterating %s - it did not complete",
- Thread.currentThread().getName(), null == interruptedTraversal ? "" : interruptedTraversal), cause);
- this.interruptedTraversal = Optional.ofNullable(interruptedTraversal);
- }
-
- public Optional<Traversal> getInterruptedTraversal() {
- return interruptedTraversal;
- }
-}
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
index 66790f7..5076072 100644
--- a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
+++ b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorTest.java
@@ -22,6 +22,8 @@ import org.apache.tinkerpop.gremlin.AbstractGremlinTest;
import org.apache.tinkerpop.gremlin.LoadGraphWith;
import org.apache.tinkerpop.gremlin.TestHelper;
import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngineTest;
+import org.apache.tinkerpop.gremlin.process.TraversalStrategies;
+import org.apache.tinkerpop.gremlin.process.graph.traversal.strategy.TimeLimitedStrategy;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.commons.lang3.concurrent.BasicThreadFactory;
import org.junit.Test;
@@ -158,71 +160,6 @@ public class GremlinExecutorTest extends AbstractGremlinTest {
}
@Test
- @LoadGraphWith(LoadGraphWith.GraphData.GRATEFUL)
- public void shouldTimeoutIteratingTraversalScript() throws Exception {
- final AtomicBoolean successCalled = new AtomicBoolean(false);
- final AtomicBoolean failureCalled = new AtomicBoolean(false);
-
- final CountDownLatch timeOutCount = new CountDownLatch(1);
-
- final GremlinExecutor gremlinExecutor = GremlinExecutor.build()
- .scriptEvaluationTimeout(1000)
- .afterFailure((b, e) -> failureCalled.set(true))
- .afterSuccess((b) -> successCalled.set(true))
- .afterTimeout((b) -> timeOutCount.countDown()).create();
- try {
- final Bindings b = new SimpleBindings();
- b.put("g", g);
- gremlinExecutor.eval("g.V().out().out().out().out().out().out().out().out().out().out().out().iterate()", b).get();
- fail("This script should have timed out with an exception");
- } catch (Exception ex) {
- assertEquals(TimeoutException.class, ex.getCause().getClass());
- }
-
- assertTrue(timeOutCount.await(2000, TimeUnit.MILLISECONDS));
-
- assertFalse(successCalled.get());
- assertFalse(failureCalled.get());
- assertEquals(0, timeOutCount.getCount());
- gremlinExecutor.close();
- }
-
- @Test
- @LoadGraphWith(LoadGraphWith.GraphData.GRATEFUL)
- public void shouldTimeoutIteratingTraversalScriptButBeSureInterruptedThreadCanBeReused() throws Exception {
- final CountDownLatch timeOutCount = new CountDownLatch(1);
-
- final ExecutorService evalExecutor = Executors.newSingleThreadExecutor(testingThreadFactory);
- final GremlinExecutor gremlinExecutor = GremlinExecutor.build()
- .executorService(evalExecutor)
- .scriptEvaluationTimeout(1000)
- .afterTimeout((b) -> timeOutCount.countDown()).create();
-
- assertEquals(2, gremlinExecutor.eval("1+1").get());
-
- try {
- final Bindings b = new SimpleBindings();
- b.put("g", g);
- gremlinExecutor.eval("g.V().out().out().out().out().out().out().out().out().out().out().out().iterate()", b).get();
- fail("This script should have timed out with an exception");
- } catch (Exception ex) {
- assertEquals(TimeoutException.class, ex.getCause().getClass());
-
- // just make sure that interrupted thread is good to go again
- assertEquals(2, gremlinExecutor.eval("1+1").get());
- assertEquals(2, gremlinExecutor.eval("1+1").get());
- assertEquals(2, gremlinExecutor.eval("1+1").get());
- }
-
- assertTrue(timeOutCount.await(2000, TimeUnit.MILLISECONDS));
-
- assertEquals(0, timeOutCount.getCount());
- gremlinExecutor.close();
- evalExecutor.shutdown();
- evalExecutor.awaitTermination(30000, TimeUnit.MILLISECONDS);
- }
-
- @Test
public void shouldCallFail() throws Exception {
final AtomicBoolean timeoutCalled = new AtomicBoolean(false);
final AtomicBoolean successCalled = new AtomicBoolean(false);
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
index 6c4ef08..2c6d45d 100644
--- a/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
+++ b/gremlin-groovy-test/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngineTest.java
@@ -26,7 +26,6 @@ import org.apache.tinkerpop.gremlin.groovy.NoImportCustomizerProvider;
import org.apache.tinkerpop.gremlin.groovy.SecurityCustomizerProvider;
import org.apache.tinkerpop.gremlin.process.T;
import org.apache.tinkerpop.gremlin.process.Traversal;
-import org.apache.tinkerpop.gremlin.process.TraversalInterruptedException;
import org.apache.tinkerpop.gremlin.structure.Direction;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.util.StreamFactory;
@@ -580,35 +579,6 @@ public class GremlinGroovyScriptEngineTest extends AbstractGremlinTest {
}
}
- @Test
- @LoadGraphWith(GRATEFUL)
- public void shouldTimeoutOnEvalOfTraversalWhereIterateHasStarted() throws Exception {
- // this is just a safety net test for groovy - see the gremlin-test/CoreTraversalTest for more
- // complete testing of breaking out of iteration
- final ScriptEngine engine = new GremlinGroovyScriptEngine();
- final Bindings b = new SimpleBindings();
- b.put("g", g);
-
- final AtomicBoolean interrupted = new AtomicBoolean(false);
-
- final Thread t = new Thread(() -> {
- try {
- engine.eval("g.V().out().out().out().out().out().out().out().out().out().out().out().iterate()", b);
- fail("No way this should have completed in any reasonable time");
- } catch (Exception ex) {
- interrupted.set(ex.getCause().getCause().getClass().equals(TraversalInterruptedException.class));
- }
- });
-
- t.start();
-
- Thread.sleep(1000);
- t.interrupt();
- t.join();
-
- assertTrue(interrupted.get());
- }
-
public static class DenyAll extends GroovyValueFilter {
@Override
public Object filter(final Object o) {
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
index 70169d2..1a03753 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutor.java
@@ -21,7 +21,6 @@ package org.apache.tinkerpop.gremlin.groovy.engine;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine;
import org.apache.tinkerpop.gremlin.groovy.plugin.GremlinPlugin;
-import org.apache.tinkerpop.gremlin.process.TraversalInterruptedException;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.commons.lang3.concurrent.BasicThreadFactory;
import org.javatuples.Pair;
@@ -169,8 +168,7 @@ public class GremlinExecutor implements AutoCloseable {
// thread interruptions will typically come as the result of a timeout, so in those cases,
// check for that situation and convert to TimeoutException
- if (root.getClass().equals(InterruptedException.class)
- || root.getClass().equals(TraversalInterruptedException.class))
+ if (root.getClass().equals(InterruptedException.class))
evaluationFuture.completeExceptionally(new TimeoutException(
String.format("Script evaluation exceeded the configured threshold of %s ms for request [%s]: %s", scriptEvaluationTimeout, script, root.getMessage())));
else {
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/CoreTraversalTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/CoreTraversalTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/CoreTraversalTest.java
index 53fd0a3..5885544 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/CoreTraversalTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/CoreTraversalTest.java
@@ -23,7 +23,6 @@ import org.apache.tinkerpop.gremlin.FeatureRequirement;
import org.apache.tinkerpop.gremlin.LoadGraphWith;
import org.apache.tinkerpop.gremlin.process.AbstractGremlinProcessTest;
import org.apache.tinkerpop.gremlin.process.Traversal;
-import org.apache.tinkerpop.gremlin.process.TraversalInterruptedException;
import org.apache.tinkerpop.gremlin.process.graph.traversal.GraphTraversal;
import org.apache.tinkerpop.gremlin.process.util.BulkSet;
import org.apache.tinkerpop.gremlin.structure.Contains;
@@ -31,18 +30,14 @@ import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Transaction;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
-import org.junit.Ignore;
import org.junit.Test;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
-import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.GRATEFUL;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
import static org.apache.tinkerpop.gremlin.process.graph.traversal.__.*;
import static org.apache.tinkerpop.gremlin.structure.Graph.Features.GraphFeatures.FEATURE_TRANSACTIONS;
@@ -206,97 +201,4 @@ public class CoreTraversalTest extends AbstractGremlinProcessTest {
assertEquals(1, IteratorUtils.count(t));
g.tx().rollback();
}
-
- @Test
- @LoadGraphWith(GRATEFUL)
- public void shouldTimeoutOnTraversalWhereIterateHasStarted() throws Exception {
- final AtomicBoolean interrupted = new AtomicBoolean(false);
-
- final Thread t = new Thread(() -> {
- try {
- g.V().out().out().out().out().out().out().out().out().out().out().out().iterate();
- fail("No way this should have completed in any reasonable time");
- } catch (Exception ex) {
- interrupted.set(ex.getClass().equals(TraversalInterruptedException.class));
- }
- });
-
- t.start();
-
- Thread.sleep(500);
- t.interrupt();
- t.join();
-
- assertTrue(interrupted.get());
- }
-
- @Test
- @LoadGraphWith(GRATEFUL)
- public void shouldTimeoutOnTraversalWhereForEachRemainingHasStarted() throws Exception {
- final AtomicBoolean interrupted = new AtomicBoolean(false);
-
- final Thread t = new Thread(() -> {
- try {
- g.V().out().out().out().out().out().out().out().out().out().out().out().forEachRemaining(Object::toString);
- fail("No way this should have completed in any reasonable time");
- } catch (Exception ex) {
- interrupted.set(ex.getClass().equals(TraversalInterruptedException.class));
- }
- });
-
- t.start();
-
- Thread.sleep(500);
- t.interrupt();
- t.join();
-
- assertTrue(interrupted.get());
- }
-
- @Test
- @LoadGraphWith(GRATEFUL)
- public void shouldTimeoutOnTraversalWhereNextingStarted() throws Exception {
- final AtomicBoolean interrupted = new AtomicBoolean(false);
-
- final Thread t = new Thread(() -> {
- try {
- g.V().out().out().out().out().out().out().out().out().out().out().out().next(Integer.MAX_VALUE);
- fail("No way this should have completed in any reasonable time");
- } catch (Exception ex) {
- ex.printStackTrace();
- interrupted.set(ex.getClass().equals(TraversalInterruptedException.class));
- }
- });
-
- t.start();
-
- Thread.sleep(500);
- t.interrupt();
- t.join();
-
- assertTrue(interrupted.get());
- }
-
- @Test
- @LoadGraphWith(GRATEFUL)
- public void shouldTimeoutOnTraversalWhereFillStarted() throws Exception {
- final AtomicBoolean interrupted = new AtomicBoolean(false);
-
- final Thread t = new Thread(() -> {
- try {
- g.V().out().out().out().out().out().out().out().out().out().out().out().fill(new ArrayList<>());
- fail("No way this should have completed in any reasonable time");
- } catch (Exception ex) {
- interrupted.set(ex.getClass().equals(TraversalInterruptedException.class));
- }
- });
-
- t.start();
-
- Thread.sleep(500);
- t.interrupt();
- t.join();
-
- assertTrue(interrupted.get());
- }
}
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/575048d0/hadoop-gremlin/src/main/java/org/apache/tinkerpop/gremlin/hadoop/structure/hdfs/HadoopElementIterator.java
----------------------------------------------------------------------
diff --git a/hadoop-gremlin/src/main/java/org/apache/tinkerpop/gremlin/hadoop/structure/hdfs/HadoopElementIterator.java b/hadoop-gremlin/src/main/java/org/apache/tinkerpop/gremlin/hadoop/structure/hdfs/HadoopElementIterator.java
index 5449c78..1951b93 100644
--- a/hadoop-gremlin/src/main/java/org/apache/tinkerpop/gremlin/hadoop/structure/hdfs/HadoopElementIterator.java
+++ b/hadoop-gremlin/src/main/java/org/apache/tinkerpop/gremlin/hadoop/structure/hdfs/HadoopElementIterator.java
@@ -22,7 +22,6 @@ import org.apache.tinkerpop.gremlin.hadoop.Constants;
import org.apache.tinkerpop.gremlin.hadoop.structure.HadoopGraph;
import org.apache.tinkerpop.gremlin.hadoop.structure.io.VertexWritable;
import org.apache.tinkerpop.gremlin.hadoop.structure.util.ConfUtil;
-import org.apache.tinkerpop.gremlin.process.TraversalInterruptedException;
import org.apache.tinkerpop.gremlin.structure.Element;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
@@ -36,7 +35,6 @@ import org.apache.hadoop.mapreduce.TaskAttemptID;
import org.apache.hadoop.mapreduce.lib.input.FileSplit;
import java.io.IOException;
-import java.nio.channels.ClosedByInterruptException;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.Queue;
@@ -69,10 +67,6 @@ public abstract class HadoopElementIterator<E extends Element> implements Iterat
this.readers.add(inputFormat.createRecordReader(new FileSplit(status.getPath(), 0, Integer.MAX_VALUE, new String[]{}), new TaskAttemptContext(configuration, new TaskAttemptID())));
}
}
- } catch (ClosedByInterruptException cbie) {
- // this is the exception that seems to rise when a Traversal interrupt occurs. Convert it to the
- // common exception expected.
- throw new TraversalInterruptedException(cbie);
} catch (Exception e) {
throw new IllegalStateException(e.getMessage(), e);
}