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 2017/07/19 19:19:24 UTC

[25/50] [abbrv] tinkerpop git commit: TINKERPOP-1714 GremlinExecutor checks for timeout from time script submitted

TINKERPOP-1714 GremlinExecutor checks for timeout from time script submitted

Prior to this change timeouts were considered from the time the script started evaluation which probably isn't in line with user expectations but also made it hard to clear the job queue on an overloaded server.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/ac19d5de
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/ac19d5de
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/ac19d5de

Branch: refs/heads/TINKERPOP-1716
Commit: ac19d5dec659a1a8f399d5a11a4812774eebeca7
Parents: 402678b
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Jul 6 11:49:21 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Jul 6 11:49:21 2017 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../gremlin/groovy/engine/GremlinExecutor.java  | 39 ++++++--------------
 2 files changed, 13 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/ac19d5de/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 96e79ab..da85ebb 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -28,6 +28,7 @@ TinkerPop 3.2.6 (Release Date: NOT OFFICIALLY RELEASED YET)
 
 This release also includes changes from <<release-3-1-8, 3.1.8>>.
 
+* `GremlinExecutor` begins timeout of script evaluation at the time the script was submitted and not from the time it began evaluation.
 * `ReferenceFactory` and `DetachedFactory` now detach elements in collections accordingly.
 * Deprecated the `useMapperFromGraph` configuration option for Gremlin Server serializers.
 * `JavaTranslator` is now smart about handling `BulkSet` and `Tree`.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/ac19d5de/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 8802793..d646a8c 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
@@ -52,9 +52,9 @@ import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -279,31 +279,6 @@ public class GremlinExecutor implements AutoCloseable {
 
         final CompletableFuture<Object> evaluationFuture = new CompletableFuture<>();
         final FutureTask<Void> evalFuture = new FutureTask<>(() -> {
-
-            if (scriptEvalTimeOut > 0) {
-                final Thread scriptEvalThread = Thread.currentThread();
-
-                logger.debug("Schedule timeout for script - {} - in thread [{}]", script, scriptEvalThread.getName());
-
-                // Schedule a timeout in the thread pool for future execution
-                final ScheduledFuture<?> sf = scheduledExecutorService.schedule(() -> {
-                    logger.warn("Timing out script - {} - in thread [{}]", script, Thread.currentThread().getName());
-                    if (!evaluationFuture.isDone()) scriptEvalThread.interrupt();
-                }, scriptEvalTimeOut, TimeUnit.MILLISECONDS);
-
-                // Cancel the scheduled timeout if the eval future is complete or the script evaluation failed
-                // with exception
-                evaluationFuture.handleAsync((v, t) -> {
-                    if (!sf.isDone()) {
-                        logger.debug("Killing scheduled timeout on script evaluation - {} - as the eval completed (possibly with exception).", script);
-                        sf.cancel(true);
-                    }
-
-                    // no return is necessary - nothing downstream is concerned with what happens in here
-                    return null;
-                }, scheduledExecutorService);
-            }
-
             try {
                 lifeCycle.getBeforeEval().orElse(beforeEval).accept(bindings);
 
@@ -349,7 +324,17 @@ public class GremlinExecutor implements AutoCloseable {
             return null;
         });
 
-        executorService.execute(evalFuture);
+        final Future<?> executionFuture = executorService.submit(evalFuture);
+        if (scriptEvalTimeOut > 0) {
+            // Schedule a timeout in the thread pool for future execution
+            scheduledExecutorService.schedule(() -> {
+                if (executionFuture.cancel(true)) {
+                    lifeCycle.getAfterTimeout().orElse(afterTimeout).accept(bindings);
+                    evaluationFuture.completeExceptionally(new TimeoutException(
+                            String.format("Script evaluation exceeded the configured 'scriptEvaluationTimeout' threshold of %s ms or evaluation was otherwise cancelled directly for request [%s]", scriptEvalTimeOut, script)));
+                }
+            }, scriptEvalTimeOut, TimeUnit.MILLISECONDS);
+        }
 
         return evaluationFuture;
     }