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 2020/10/08 20:27:17 UTC
[tinkerpop] branch TINKERPOP-2438 created (now 9d8d0ad)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2438
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at 9d8d0ad TINKERPOP-2438 wip
This branch includes the following new commits:
new 9d8d0ad TINKERPOP-2438 wip
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[tinkerpop] 01/01: TINKERPOP-2438 wip
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2438
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 9d8d0adaed89a70501615dfdb9266158e5699d35
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Thu Oct 8 16:26:32 2020 -0400
TINKERPOP-2438 wip
---
.../groovy/jsr223/ast/GremlinASTChecker.groovy | 111 +++++++++++++++++++++
.../gremlin/groovy/engine/GremlinExecutor.java | 8 +-
.../engine/GremlinExecutorOverGraphTest.java | 32 ++++++
.../groovy/jsr223/ast/GremlinASTCheckerTest.java | 70 +++++++++++++
4 files changed, 219 insertions(+), 2 deletions(-)
diff --git a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/jsr223/ast/GremlinASTChecker.groovy b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/jsr223/ast/GremlinASTChecker.groovy
new file mode 100644
index 0000000..5ce361c
--- /dev/null
+++ b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/jsr223/ast/GremlinASTChecker.groovy
@@ -0,0 +1,111 @@
+/*
+ * 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.groovy.jsr223.ast
+
+import org.codehaus.groovy.ast.CodeVisitorSupport
+import org.codehaus.groovy.ast.builder.AstBuilder
+import org.codehaus.groovy.ast.expr.ArgumentListExpression
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codehaus.groovy.ast.expr.PropertyExpression
+import org.codehaus.groovy.ast.expr.VariableExpression
+import org.codehaus.groovy.control.CompilePhase
+
+/**
+ * Processes Gremlin strings as an AST so as to try to detect certain properties from the script without actual
+ * having to execute a {@code eval()} on it.
+ */
+final class GremlinASTChecker {
+
+ private static final astBuilder = new AstBuilder()
+
+ /**
+ * Parses a Gremlin script and extracts a {@code Result} containing properties that are relevant to the checker.
+ */
+ static Result parse(String gremlin) {
+ def ast = astBuilder.buildFromString(CompilePhase.CONVERSION, false, gremlin)
+ def tocheck = new TimeoutCheck()
+ ast[0].visit(tocheck)
+ return new Result(tocheck.total)
+ }
+
+ static class Result {
+ private final long timeout
+
+ Result(long timeout) {
+ this.timeout = timeout
+ }
+
+ /**
+ * Gets the value of the timeouts that were set using the {@code with()} source step. If there are multiple
+ * commands using this step, the timeouts are summed together.
+ */
+ Optional<Long> getTimeout() {
+ timeout == 0 ? Optional.empty() : Optional.of(timeout)
+ }
+ }
+
+ static class TimeoutCheck extends CodeVisitorSupport {
+
+ def total = 0L
+
+ @Override
+ void visitMethodCallExpression(MethodCallExpression call) {
+ if (call.methodAsString == "with")
+ call.getArguments().visit(this)
+
+ call.getObjectExpression().visit(this);
+ }
+
+ @Override
+ void visitArgumentlistExpression(ArgumentListExpression ale) {
+ def argumentExpressions = ale.asList()
+ if (argumentExpressions.size() == 2) {
+ def key = getArgument(argumentExpressions[0])
+ def value = getArgument(argumentExpressions[1])
+
+ if (isTimeoutKey(key) && value instanceof Number) {
+ total += value
+ }
+ }
+ }
+
+ private static isTimeoutKey(def k) {
+ // can't use Tokens object from here as it's in gremlin-driver. would rather not put a
+ // groovy build in gremlin-driver so we're just stuck with strings for now.
+ return k == "evaluationTimeout" ||
+ k == "scriptEvaluationTimeout" ||
+ k == "ARGS_EVAL_TIMEOUT" ||
+ k == "ARGS_SCRIPT_EVAL_TIMEOUT"
+ }
+
+ private static getArgument(Expression expr) {
+ // ConstantExpression for a direct String usage,
+ // PropertyExpression for Token.*
+ // VariableExpression for static import of the Token field
+ if (expr instanceof ConstantExpression)
+ return ((ConstantExpression) expr).value
+ else if (expr instanceof PropertyExpression)
+ return ((PropertyExpression) expr).propertyAsString
+ else if (expr instanceof VariableExpression)
+ return ((VariableExpression) expr).text
+ }
+ }
+}
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 f8c0a81..ad11748 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,6 +21,7 @@ package org.apache.tinkerpop.gremlin.groovy.engine;
import org.apache.commons.lang3.ClassUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.commons.lang3.concurrent.BasicThreadFactory;
+import org.apache.tinkerpop.gremlin.groovy.jsr223.ast.GremlinASTChecker;
import org.apache.tinkerpop.gremlin.jsr223.CachedGremlinScriptEngineManager;
import org.apache.tinkerpop.gremlin.jsr223.ConcurrentBindings;
import org.apache.tinkerpop.gremlin.jsr223.GremlinPlugin;
@@ -255,8 +256,11 @@ public class GremlinExecutor implements AutoCloseable {
bindings.putAll(globalBindings);
bindings.putAll(boundVars);
- // override the timeout if the lifecycle has a value assigned
- final long scriptEvalTimeOut = lifeCycle.getEvaluationTimeoutOverride().orElse(evaluationTimeout);
+ // override the timeout if the lifecycle has a value assigned. if the script contains with(timeout)
+ // options then allow that value to override what's provided on the lifecycle
+ final Optional<Long> timeoutDefinedInScript = GremlinASTChecker.parse(script).getTimeout();
+ final long scriptEvalTimeOut = timeoutDefinedInScript.orElse(
+ lifeCycle.getEvaluationTimeoutOverride().orElse(evaluationTimeout));
final CompletableFuture<Object> evaluationFuture = new CompletableFuture<>();
final FutureTask<Void> evalFuture = new FutureTask<>(() -> {
diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorOverGraphTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorOverGraphTest.java
index 295ca74..72f4fd5 100644
--- a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorOverGraphTest.java
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/GremlinExecutorOverGraphTest.java
@@ -23,17 +23,22 @@ import org.apache.tinkerpop.gremlin.LoadGraphWith;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
+import org.hamcrest.Matchers;
import org.junit.Test;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import static org.hamcrest.core.StringStartsWith.startsWith;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
/**
* @author Stephen Mallette (http://stephen.genoprime.com)
@@ -42,6 +47,33 @@ public class GremlinExecutorOverGraphTest {
private final BasicThreadFactory testingThreadFactory = new BasicThreadFactory.Builder().namingPattern("test-gremlin-executor-%d").build();
@Test
+ public void shouldOverrideTimeoutWithinScript() throws Exception {
+ final TinkerGraph graph = TinkerFactory.createModern();
+ final GraphTraversalSource g = graph.traversal();
+
+ final ExecutorService evalExecutor = Executors.newSingleThreadExecutor(testingThreadFactory);
+ final GremlinExecutor gremlinExecutor = GremlinExecutor.build()
+ .evaluationTimeout(60000)
+ .afterSuccess(b -> {
+ final GraphTraversalSource ig = (GraphTraversalSource) b.get("g");
+ if (ig.getGraph().features().graph().supportsTransactions())
+ ig.tx().commit();
+ })
+ .executorService(evalExecutor).create();
+
+ final Map<String,Object> bindings = new HashMap<>();
+ bindings.put("g", g);
+
+ try {
+ gremlinExecutor.eval("g.with('evaluationTimeout',100).V().repeat(both())").get();
+ fail("Should have timedout");
+ } catch (ExecutionException ex) {
+ // should die in 100ms not 60000
+ assertThat(ex.getCause().getMessage(), startsWith("Evaluation exceeded the configured 'evaluationTimeout' threshold of 100 ms"));
+ }
+ }
+
+ @Test
public void shouldAllowTraversalToIterateInDifferentThreadThanOriginallyEvaluatedWithAutoCommit() throws Exception {
final TinkerGraph graph = TinkerFactory.createModern();
final GraphTraversalSource g = graph.traversal();
diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ast/GremlinASTCheckerTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ast/GremlinASTCheckerTest.java
new file mode 100644
index 0000000..bd29589
--- /dev/null
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ast/GremlinASTCheckerTest.java
@@ -0,0 +1,70 @@
+/*
+ * 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.groovy.jsr223.ast;
+
+import org.junit.Test;
+
+import java.util.Optional;
+
+import static org.junit.Assert.assertEquals;
+
+public class GremlinASTCheckerTest {
+
+ @Test
+ public void shouldNotFindTimeout() {
+ assertEquals(Optional.empty(), GremlinASTChecker.parse("g.with(true).V().out('knows')").getTimeout());
+ }
+
+ @Test
+ public void shouldNotFindTimeoutCozWeCommentedItOut() {
+ assertEquals(Optional.empty(), GremlinASTChecker.parse("g.\n" +
+ " // with('evaluationTimeout', 1000L).\n" +
+ " with(true).V().out('knows')").getTimeout());
+ }
+
+ @Test
+ public void shouldIdentifyTimeoutAsStringKey() {
+ assertEquals(1000, GremlinASTChecker.parse("g.with('evaluationTimeout', 1000L).with(true).V().out('knows')").
+ getTimeout().get().longValue());
+ }
+
+ @Test
+ public void shouldIdentifyTimeoutAsTokenKey() {
+ assertEquals(1000, GremlinASTChecker.parse("g.with(Tokens.ARGS_EVAL_TIMEOUT, 1000L).with(true).V().out('knows')").
+ getTimeout().get().longValue());
+ }
+
+ @Test
+ public void shouldIdentifyTimeoutAsTokenKeyWithoutClassName() {
+ assertEquals(1000, GremlinASTChecker.parse("g.with(ARGS_EVAL_TIMEOUT, 1000L).with(true).V().out('knows')").
+ getTimeout().get().longValue());
+ }
+
+ @Test
+ public void shouldIdentifyMultipleTimeouts() {
+ assertEquals(6000, GremlinASTChecker.parse("g.with('evaluationTimeout', 1000L).with(true).V().out('knows');" +
+ "g.with('evaluationTimeout', 1000L).with(true).V().out('knows')\n" +
+ " //g.with('evaluationTimeout', 1000L).with(true).V().out('knows')\n" +
+ " g.with('evaluationTimeout', 1000L).with(true).V().out('knows')\n" +
+ " g.with(Tokens.ARGS_SCRIPT_EVAL_TIMEOUT, 1000L).with(true).V().out('knows')\n" +
+ " g.with(ARGS_EVAL_TIMEOUT, 1000L).with(true).V().out('knows')\n" +
+ " g.with('scriptEvaluationTimeout', 1000L).with(true).V().out('knows')").
+ getTimeout().get().longValue());
+ }
+}