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/23 23:39:50 UTC
tinkerpop git commit: Simplified Gremlin Server error handling CTR
Repository: tinkerpop
Updated Branches:
refs/heads/tp32 3f06a6a5d -> a7e0e0f50
Simplified Gremlin Server error handling CTR
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/a7e0e0f5
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/a7e0e0f5
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/a7e0e0f5
Branch: refs/heads/tp32
Commit: a7e0e0f5008b20e2d16ecfa2e4890468ef2a53a7
Parents: 3f06a6a
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Sun Jul 23 18:47:54 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Sun Jul 23 18:47:54 2017 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../gremlin/groovy/engine/GremlinExecutor.java | 1 -
.../server/op/AbstractEvalOpProcessor.java | 29 +++-----------
.../server/GremlinDriverIntegrateTest.java | 40 +++++++++++++++++++-
4 files changed, 44 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a7e0e0f5/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 3a0627d..9039d9f 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>>.
+* Exceptions that occur during result iteration in Gremlin Server will now return `SCRIPT_EVALUATION_EXCEPTION` rather than `SERVER_ERROR`.
* Initialization scripts for Gremlin Server will not timeout.
* Added Gremlin.Net.
* `ProfileTest` is now less stringent about assertions which will reduce burdens on providers.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a7e0e0f5/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 962f798..fabc8cb 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
@@ -331,7 +331,6 @@ public class GremlinExecutor implements AutoCloseable {
// Schedule a timeout in the thread pool for future execution
scheduledExecutorService.schedule(() -> {
if (executionFuture.cancel(true)) {
- lifeCycle.getAfterTimeout().orElse(afterTimeout).accept(bindings);
final CompletableFuture<Object> ef = evaluationFutureRef.get();
if (ef != null) {
ef.completeExceptionally(new TimeoutException(
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a7e0e0f5/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
index a76323f..46b3c8d 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
@@ -266,31 +266,12 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
try {
handleIterator(context, itty);
- } catch (TimeoutException ex) {
- // a timeout occurs if serializedResponseTimeout is exceeded, but that setting is deprecated
- // and by default disabled. we can remove this handling once we drop that setting all together
- final String errorMessage = String.format("Response iteration exceeded the configured threshold for request [%s] - %s", msg, ex.getMessage());
- logger.warn(errorMessage);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
- .statusMessage(errorMessage)
- .statusAttributeException(ex).create());
- if (managedTransactionsForRequest) attemptRollback(msg, context.getGraphManager(), settings.strictTransactionManagement);
- } catch (InterruptedException ex) {
- // interruption occurs if there is a forced timeout during result iteration. this timeout
- // is driven by the script evaluation timeout so for consistency the message should be the same
- final String errorMessage = String.format("Script evaluation exceeded the configured threshold for request [%s] - %s", msg, ex.getMessage());
- logger.warn(errorMessage, ex);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
- .statusMessage(ex.getMessage())
- .statusAttributeException(ex).create());
- if (managedTransactionsForRequest) attemptRollback(msg, context.getGraphManager(), settings.strictTransactionManagement);
} catch (Exception ex) {
- logger.warn(String.format("Exception processing a script on request [%s].", msg), ex);
- final String err = ex.getMessage();
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR)
- .statusMessage(null == err || err.isEmpty() ? ex.getClass().getSimpleName() : err)
- .statusAttributeException(ex).create());
if (managedTransactionsForRequest) attemptRollback(msg, context.getGraphManager(), settings.strictTransactionManagement);
+
+ // wrap up the exception and rethrow. the error will be written to the client by the evalFuture
+ // as it will completeExceptionally in the GremlinExecutor
+ throw new RuntimeException(ex);
}
}).create();
@@ -317,7 +298,7 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
.statusMessage("Timeout during script evaluation triggered by TimedInterruptCustomizerProvider")
.statusAttributeException(t).create());
} else if (t instanceof TimeoutException) {
- final String errorMessage = String.format("Script evaluation exceeded the configured threshold for request [%s] - %s", msg, t.getMessage());
+ final String errorMessage = String.format("Script evaluation exceeded the configured threshold for request [%s]", msg);
logger.warn(errorMessage, t);
ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
.statusMessage(t.getMessage())
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a7e0e0f5/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
index 6d4f236..9662ab7 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
@@ -171,12 +171,48 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
settings.scriptEvaluationTimeout = 250;
settings.threadPoolWorker = 1;
break;
+ case "shouldProcessTraversalInterruption":
+ case "shouldProcessEvalInterruption":
+ settings.scriptEvaluationTimeout = 1500;
+ break;
}
return settings;
}
@Test
+ public void shouldProcessTraversalInterruption() throws Exception {
+ final Cluster cluster = TestClientFactory.open();
+ final Client client = cluster.connect();
+
+ try {
+ client.submit("g.inject(1).sideEffect{Thread.sleep(5000)}").all().get();
+ fail("Should have timed out");
+ } catch (Exception ex) {
+ final ResponseException re = (ResponseException) ex.getCause();
+ assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, re.getResponseStatusCode());
+ }
+
+ cluster.close();
+ }
+
+ @Test
+ public void shouldProcessEvalInterruption() throws Exception {
+ final Cluster cluster = TestClientFactory.open();
+ final Client client = cluster.connect();
+
+ try {
+ client.submit("Thread.sleep(5000);'done'").all().get();
+ fail("Should have timed out");
+ } catch (Exception ex) {
+ final ResponseException re = (ResponseException) ex.getCause();
+ assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, re.getResponseStatusCode());
+ }
+
+ cluster.close();
+ }
+
+ @Test
public void shouldKeepAliveForWebSockets() throws Exception {
// keep the connection pool size at 1 to remove the possibility of lots of connections trying to ping which will
// complicate the assertion logic
@@ -1189,7 +1225,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(ResponseException.class));
final ResponseException re = (ResponseException) root;
- assertEquals(ResponseStatusCode.SERVER_ERROR, re.getResponseStatusCode());
+ assertEquals(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION, re.getResponseStatusCode());
}
// keep the testing here until "rebind" is completely removed
@@ -1245,7 +1281,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
final Throwable root = ExceptionUtils.getRootCause(ex);
assertThat(root, instanceOf(ResponseException.class));
final ResponseException re = (ResponseException) root;
- assertEquals(ResponseStatusCode.SERVER_ERROR, re.getResponseStatusCode());
+ assertEquals(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION, re.getResponseStatusCode());
}
// keep the testing here until "rebind" is completely removed