Repository: tinkerpop Updated Branches: refs/heads/master 1526c2c55 -> 65d7a82ff
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/master 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; }
