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 2016/06/30 21:16:59 UTC

[3/3] tinkerpop git commit: Fixed bug in driver where the client would unecessarily replace Connections.

Fixed bug in driver where the client would unecessarily replace Connections.

Replacing the Connection was pretty transparent to the user in most cases, but it wasn't a good practice as there was a cost in doing that which was uncessary since the pool size for a session is always 1.


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

Branch: refs/heads/TINKERPOP-1352
Commit: 989977f67c42d9ab44db3b055db907cd8ee251b0
Parents: bc397ec
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Jun 30 17:14:57 2016 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Jun 30 17:14:57 2016 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |    1 +
 .../tinkerpop/gremlin/driver/Connection.java    |    5 +-
 .../gremlin/driver/ConnectionPool.java          |    9 +-
 .../gremlin/server/op/session/Session.java      |    4 +
 .../server/op/session/SessionOpProcessor.java   |    1 -
 .../server/GremlinDriverIntegrateTest.java      | 2070 +++++++++---------
 .../GremlinServerSessionIntegrateTest.java      |   17 +-
 7 files changed, 1058 insertions(+), 1049 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/989977f6/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index f2d4153..7cf46f8 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -31,6 +31,7 @@ TinkerPop 3.1.3 (NOT OFFICIALLY RELEASED YET)
 * Defaulted to `Edge.DEFAULT` if no edge label was supplied in GraphML.
 * Fixed bug in `IoGraphTest` causing IllegalArgumentException: URI is not hierarchical error for external graph implementations.
 * Fixed a bug where timeout functions provided to the `GremlinExecutor` were not executing in the same thread as the script evaluation.
+* Fixed a bug in the driver where many parallel requests over a session would sometimes force a connection to close and replace itself.
 * Optimized a few special cases in `RangeByIsCountStrategy`.
 * Fixed a bug where the `ConnectionPool` in the driver would not grow with certain configuration options.
 * Fixed a bug where pauses in Gremlin Server writing to an overtaxed client would generate unexpected `FastNoSuchElementException` errors.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/989977f6/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
index cecfbc5..22e48fe 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Connection.java
@@ -19,7 +19,6 @@
 package org.apache.tinkerpop.gremlin.driver;
 
 import io.netty.handler.codec.CodecException;
-import io.netty.handler.codec.CorruptedFrameException;
 import org.apache.tinkerpop.gremlin.driver.exception.ConnectionException;
 import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
 import io.netty.bootstrap.Bootstrap;
@@ -120,7 +119,9 @@ final class Connection {
      * the maximum number of in-process requests less the number of pending responses.
      */
     public int availableInProcess() {
-        return maxInProcess - pending.size();
+        // no need for a negative available amount - not sure that the pending size can ever exceed maximum, but
+        // better to avoid the negatives that would ensue if it did
+        return Math.max(0, maxInProcess - pending.size());
     }
 
     public boolean isDead() {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/989977f6/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java
index 52c6b6a..e51662e 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java
@@ -180,8 +180,10 @@ final class ConnectionPool {
 
             // destroy a connection that exceeds the minimum pool size - it does not have the right to live if it
             // isn't busy. replace a connection that has a low available in process count which likely means that
-            // it's backing up with requests that might never have returned. if neither of these scenarios are met
-            // then let the world know the connection is available.
+            // it's backing up with requests that might never have returned. consider the maxPoolSize in this condition
+            // because if it is equal to 1 (which it is for a session) then there is no need to replace the connection
+            // as it will be responsible for every single request. if neither of these scenarios are met then let the
+            // world know the connection is available.
             final int poolSize = connections.size();
             final int availableInProcess = connection.availableInProcess();
             if (poolSize > minPoolSize && borrowed <= minSimultaneousUsagePerConnection) {
@@ -189,7 +191,7 @@ final class ConnectionPool {
                     logger.debug("On {} pool size of {} > minPoolSize {} and borrowed of {} <= minSimultaneousUsagePerConnection {} so destroy {}",
                             host, poolSize, minPoolSize, borrowed, minSimultaneousUsagePerConnection, connection.getConnectionInfo());
                 destroyConnection(connection);
-            } else if (availableInProcess < minInProcess) {
+            } else if (availableInProcess < minInProcess && maxPoolSize > 1) {
                 if (logger.isDebugEnabled())
                     logger.debug("On {} availableInProcess {} < minInProcess {} so replace {}", host, availableInProcess, minInProcess, connection.getConnectionInfo());
                 replaceConnection(connection);
@@ -243,7 +245,6 @@ final class ConnectionPool {
     void replaceConnection(final Connection connection) {
         logger.debug("Replace {}", connection);
 
-        open.decrementAndGet();
         considerNewConnection();
         definitelyDestroyConnection(connection);
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/989977f6/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
index 0ed2041..33b2752 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
@@ -100,6 +100,10 @@ public class Session {
         return executor;
     }
 
+    public String getSessionId() {
+        return session;
+    }
+
     public void touch() {
         // if the task of killing is cancelled successfully then reset the session monitor. otherwise this session
         // has already been killed and there's nothing left to do with this session.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/989977f6/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
index 451c479..3497169 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
@@ -36,7 +36,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.script.Bindings;
-import javax.script.SimpleBindings;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;