tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject tinkerpop git commit: TINKERPOP-1350 was never quite fixed in 3.1.3.
Date Mon, 08 Aug 2016 10:50:49 GMT
Repository: tinkerpop
Updated Branches:
  refs/heads/tp31 87960e7ce -> 58d8bade7


TINKERPOP-1350 was never quite fixed in 3.1.3.

Changed response encoding to not use the session executor when the session has an error condition
it is trying to serialize. This should be fine as there is no need to serialized an error
condition as part of a transaction and thus no need to have the session thread to do it. That
in turn frees up the worker executor to serialize and cancel long run jobs in the session.
Removed recommendations for submitting parallel requests on a session from docs.


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

Branch: refs/heads/tp31
Commit: 58d8bade7425c7a7865382990eaaed2b7d90659c
Parents: 87960e7
Author: Stephen Mallette <spmva@genoprime.com>
Authored: Tue Aug 2 15:59:19 2016 -0400
Committer: Stephen Mallette <spmva@genoprime.com>
Committed: Mon Aug 8 06:49:17 2016 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../src/reference/gremlin-applications.asciidoc |  6 +-
 .../upgrade/release-3.1.x-incubating.asciidoc   | 22 +++++-
 .../gremlin/groovy/engine/GremlinExecutor.java  |  2 -
 .../handler/GremlinResponseFrameEncoder.java    | 13 +++-
 .../server/GremlinDriverIntegrateTest.java      | 81 +++++++++++---------
 6 files changed, 75 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/58d8bade/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 306185c..975f6cf 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -28,6 +28,7 @@ TinkerPop 3.1.4 (NOT OFFICIALLY RELEASED YET)
 
 * Fixed a potential leak of a `ReferenceCounted` resource in Gremlin Server.
 * Renamed distributions to make the prefix "apache-tinkerpop-" as opposed to just "apache-".
+* Fixed a problem (previously thought resolved on 3.1.3) causing Gremlin Server to lock up
when parallel requests were submitted on the same session if those parallel requests included
a script that blocked indefinitely.
 
 [[release-3-1-3]]
 TinkerPop 3.1.3 (Release Date: July 18, 2016)

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/58d8bade/docs/src/reference/gremlin-applications.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc
index 9f5121f..cedd98f 100644
--- a/docs/src/reference/gremlin-applications.asciidoc
+++ b/docs/src/reference/gremlin-applications.asciidoc
@@ -1261,7 +1261,7 @@ Tuning
 image:gremlin-handdrawn.png[width=120,float=right] Tuning Gremlin Server for a particular
environment may require some simple trial-and-error, but the following represent some basic
guidelines that might be useful:
 
 * Gremlin Server defaults to a very modest maximum heap size.  Consider increasing this value
for non-trivial uses.  Maximum heap size (`-Xmx`) is defined with the `JAVA_OPTIONS` setting
in `gremlin-server.sh`.
-* When configuring the size of `threadPoolWorker` start with the default of `1` and increment
by one as needed to a maximum of `2*number of cores`. Note that if using sessions that will
accept parallel requests on the same session, then this value should be no less than `2`.
+* When configuring the size of `threadPoolWorker` start with the default of `1` and increment
by one as needed to a maximum of `2*number of cores`.
 * The "right" size of the `gremlinPool` setting is somewhat dependent on the type of scripts
that will be processed
 by Gremlin Server.  As requests arrive to Gremlin Server they are decoded and queued to be
processed by threads in
 this pool.  When this pool is exhausted of threads, Gremlin Server will continue to accept
incoming requests, but
@@ -1407,10 +1407,6 @@ request.
 A session is a "heavier" approach to the simple "request/response" approach of sessionless
requests, but is sometimes
 necessary for a given use case.
 
-IMPORTANT: If submitting requests in parallel to a single session in Gremlin Server, then
the `threadPoolWorker`
-setting can be no less than `2` or else the session may be prone to becoming locked if scripts
sent on that session
-tend to block for extended periods of time.
-
 [[considering-transactions]]
 Considering Transactions
 ^^^^^^^^^^^^^^^^^^^^^^^^

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/58d8bade/docs/src/upgrade/release-3.1.x-incubating.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/upgrade/release-3.1.x-incubating.asciidoc b/docs/src/upgrade/release-3.1.x-incubating.asciidoc
index 11cdb32..b4a1657 100644
--- a/docs/src/upgrade/release-3.1.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.1.x-incubating.asciidoc
@@ -22,6 +22,26 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 
 *A 187 On The Undercover Gremlinz*
 
+TinkerPop 3.1.4
+---------------
+
+*Release Date: NOT OFFICIALLY RELEASED YET*
+
+Please see the link:https://github.com/apache/tinkerpop/blob/3.1.4/CHANGELOG.asciidoc#tinkerpop-314-release-date-XXXXXXX-XX-2016[changelog]
for a complete list of all the modifications that are part of this release.
+
+Upgrading for Users
+~~~~~~~~~~~~~~~~~~~
+
+Gremlin Server Workers
+^^^^^^^^^^^^^^^^^^^^^^
+
+In release 3.1.3, a link:http://tinkerpop.apache.org/docs/3.1.3/upgrade/#_tinkerpop_3_1_3[recommendation]
was made to
+ensure that the `threadPoolWorker` setting for Gremlin Server was no less than `2` in cases
where Gremlin Server was
+being used with sessions that accept parallel requests. In 3.1.4, that is no longer the case
and a size of `1` remains
+acceptable even in that specific case.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-1350[TINKERPOP-1350]
+
 TinkerPop 3.1.3
 ---------------
 
@@ -74,8 +94,6 @@ those that block for an extended period of time) may cause Gremlin Server
to loc
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-1350[TINKERPOP-1350]
 
-
-
 Upgrading for Providers
 ~~~~~~~~~~~~~~~~~~~~~~~
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/58d8bade/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 d70ff3d..da12e1e 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
@@ -232,8 +232,6 @@ public class GremlinExecutor implements AutoCloseable {
         return eval(script, language, boundVars, lifeCycle);
     }
 
-    private static final AtomicInteger ugh = new AtomicInteger(0);
-
     /**
      * Evaluate a script and allow for the submission of alteration to the entire evaluation
execution lifecycle.
      *

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/58d8bade/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/GremlinResponseFrameEncoder.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/GremlinResponseFrameEncoder.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/GremlinResponseFrameEncoder.java
index 50c177b..d0f9d76 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/GremlinResponseFrameEncoder.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/GremlinResponseFrameEncoder.java
@@ -61,8 +61,12 @@ public class GremlinResponseFrameEncoder extends MessageToMessageEncoder<Respons
             if (useBinary) {
                 final Frame serialized;
 
-                // if the request came in on a session then the serialization must occur
in that same thread.
-                if (null == session)
+                // if the request came in on a session then the serialization must occur
in that same thread, except
+                // in the case of an error where we can free the session executor from having
to do that job. the
+                // problem here is that if the session executor is used in the case of an
error and the executor is
+                // blocked by parallel requests then there is no thread available to serialize
the result and send
+                // back the response as the workers get all tied up behind the session executor.
+                if (null == session || !o.getStatus().getCode().isSuccess())
                     serialized = new Frame(serializer.serializeResponseAsBinary(o, ctx.alloc()));
                 else
                     serialized = new Frame(session.getExecutor().submit(() -> serializer.serializeResponseAsBinary(o,
ctx.alloc())).get());
@@ -75,8 +79,9 @@ public class GremlinResponseFrameEncoder extends MessageToMessageEncoder<Respons
 
                 final Frame serialized;
 
-                // if the request came in on a session then the serialization must occur
in that same thread.
-                if (null == session)
+                // if the request came in on a session then the serialization must occur
that same thread except
+                // in the case of errors for reasons described above.
+                if (null == session || !o.getStatus().getCode().isSuccess())
                     serialized = new Frame(textSerializer.serializeResponseAsString(o));
                 else
                     serialized = new Frame(session.getExecutor().submit(() -> textSerializer.serializeResponseAsString(o)).get());

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/58d8bade/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 7f74e03..7314243 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
@@ -125,8 +125,8 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
                 settings.graphs.put("graph", "conf/neo4j-empty.properties");
                 break;
             case "shouldProcessSessionRequestsInOrderAfterTimeout":
-                settings.scriptEvaluationTimeout = 1000;
-                settings.threadPoolWorker = 2;
+                settings.scriptEvaluationTimeout = 250;
+                settings.threadPoolWorker = 1;
                 break;
         }
 
@@ -1210,48 +1210,55 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
         final Cluster cluster = Cluster.open();
         final Client client = cluster.connect(name.getMethodName());
 
-        final ResultSet first = client.submit(
-                "Object mon1 = 'mon1';\n" +
-                        "synchronized (mon1) {\n" +
-                        "    mon1.wait();\n" +
-                        "} ");
-
-        final ResultSet second = client.submit(
-                "Object mon2 = 'mon2';\n" +
-                        "synchronized (mon2) {\n" +
-                        "    mon2.wait();\n" +
-                        "}");
-
-        final CompletableFuture<List<Result>> futureFirst = first.all();
-        final CompletableFuture<List<Result>> futureSecond = second.all();
-
-        final AtomicBoolean hit = new AtomicBoolean(false);
-        while (!futureFirst.isDone()) {
-            // futureSecond can't finish before futureFirst - racy business here?
-            assertThat(futureSecond.isDone(), is(false));
-            hit.set(true);
+        for(int index = 0; index < 50; index++)
+        {
+            final CompletableFuture<ResultSet> first = client.submitAsync(
+                    "Object mon1 = 'mon1';\n" +
+                            "synchronized (mon1) {\n" +
+                            "    mon1.wait();\n" +
+                            "} ");
+
+            final CompletableFuture<ResultSet> second = client.submitAsync(
+                    "Object mon2 = 'mon2';\n" +
+                            "synchronized (mon2) {\n" +
+                            "    mon2.wait();\n" +
+                            "}");
+
+            final CompletableFuture<ResultSet> third = client.submitAsync(
+                    "Object mon3 = 'mon3';\n" +
+                            "synchronized (mon3) {\n" +
+                            "    mon3.wait();\n" +
+                            "}");
+
+            final CompletableFuture<ResultSet> fourth = client.submitAsync(
+                    "Object mon4 = 'mon4';\n" +
+                            "synchronized (mon4) {\n" +
+                            "    mon4.wait();\n" +
+                            "}");
+
+            final CompletableFuture<List<Result>> futureFirst = first.get().all();
+            final CompletableFuture<List<Result>> futureSecond = second.get().all();
+            final CompletableFuture<List<Result>> futureThird = third.get().all();
+            final CompletableFuture<List<Result>> futureFourth = fourth.get().all();
+
+            assertFutureTimeout(futureFirst);
+            assertFutureTimeout(futureSecond);
+            assertFutureTimeout(futureThird);
+            assertFutureTimeout(futureFourth);
         }
+    }
 
-        // should have entered the loop at least once and thus proven that futureSecond didn't
return ahead of
-        // futureFirst
-        assertThat(hit.get(), is(true));
-
-        try {
+    private void assertFutureTimeout(final CompletableFuture<List<Result>> futureFirst)
{
+        try
+        {
             futureFirst.get();
             fail("Should have timed out");
-        } catch (Exception ex) {
-            final Throwable root = ExceptionUtils.getRootCause(ex);
-            assertThat(root, instanceOf(ResponseException.class));
-            assertThat(root.getMessage(), startsWith("Script evaluation exceeded the configured
'scriptEvaluationTimeout' threshold of 1000 ms for request"));
         }
-
-        try {
-            futureSecond.get();
-            fail("Should have timed out");
-        } catch (Exception ex) {
+        catch (Exception ex)
+        {
             final Throwable root = ExceptionUtils.getRootCause(ex);
             assertThat(root, instanceOf(ResponseException.class));
-            assertThat(root.getMessage(), startsWith("Script evaluation exceeded the configured
'scriptEvaluationTimeout' threshold of 1000 ms for request"));
+            assertThat(root.getMessage(), startsWith("Script evaluation exceeded the configured
'scriptEvaluationTimeout' threshold of 250 ms for request"));
         }
     }
 }


Mime
View raw message