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;
