Repository: tinkerpop Updated Branches: refs/heads/tp31 bb9d718cc -> c8c77afeb
Fixed logic in ConnectionPool that was preventing it from growing. Also, fixed a bug (an invalid FastNoSuchElementException) in Gremlin Server that would only appear when the server paused writes to an overwhelmed client. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/bc397eca Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/bc397eca Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/bc397eca Branch: refs/heads/tp31 Commit: bc397eca68caa9b0fb41f6146bd610bf6530c9ab Parents: d699cb6 Author: Stephen Mallette <[email protected]> Authored: Wed Jun 29 15:14:00 2016 -0400 Committer: Stephen Mallette <[email protected]> Committed: Wed Jun 29 15:14:00 2016 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 2 ++ .../apache/tinkerpop/gremlin/driver/ConnectionPool.java | 10 +--------- gremlin-server/conf/gremlin-server-neo4j.yaml | 2 +- gremlin-server/conf/gremlin-server-rest-secure.yaml | 2 +- gremlin-server/conf/gremlin-server-secure.yaml | 2 +- gremlin-server/conf/gremlin-server-spark.yaml | 2 +- gremlin-server/conf/gremlin-server.yaml | 2 +- .../gremlin/server/op/AbstractEvalOpProcessor.java | 11 ++++++++++- .../gremlin/server/gremlin-server-integration.yaml | 2 +- .../gremlin/server/gremlin-server-performance.yaml | 2 +- 10 files changed, 20 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index a359eca..f2d4153 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,8 @@ TinkerPop 3.1.3 (NOT OFFICIALLY RELEASED YET) * 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. * 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. * Named the thread pool used by Gremlin Server sessions: "gremlin-server-session-$n". * Fixed a bug in `BulkSet.equals()` which made itself apparent when using `store()` and `aggregate()` with labeled `cap()`. * Fixed a bug where `Result.one()` could potentially block indefinitely under certain circumstances. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/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 aab9646..52c6b6a 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 @@ -148,15 +148,7 @@ final class ConnectionPool { final int borrowed = leastUsedConn.borrowed.get(); final int availableInProcess = leastUsedConn.availableInProcess(); - // if the number borrowed starts to exceed what's available for this connection, then we need - // to wait for a connection to become available. this is an interesting comparison for "busy-ness" - // because it compares the number of times the connection was borrowed to what's in-process. the - // in-process number refers to the number of outstanding requests less the maxInProcessForConnection - // setting. this scenario can only really happen if - // maxInProcessForConnection=maxSimultaneousUsagePerConnection or if there is some sort of batch type - // operation where more than one message is sent on a single borrowed connection before it is returned - // to the pool. - if (borrowed >= leastUsedConn.availableInProcess()) { + if (borrowed >= maxSimultaneousUsagePerConnection && leastUsedConn.availableInProcess() == 0) { logger.debug("Least used connection selected from pool for {} but borrowed [{}] >= availableInProcess [{}] - wait", host, borrowed, availableInProcess); return waitForConnection(timeout, unit); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/conf/gremlin-server-neo4j.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/conf/gremlin-server-neo4j.yaml b/gremlin-server/conf/gremlin-server-neo4j.yaml index e9d6cd7..ca2d99c 100644 --- a/gremlin-server/conf/gremlin-server-neo4j.yaml +++ b/gremlin-server/conf/gremlin-server-neo4j.yaml @@ -66,7 +66,7 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536 ssl: { enabled: false} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/conf/gremlin-server-rest-secure.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/conf/gremlin-server-rest-secure.yaml b/gremlin-server/conf/gremlin-server-rest-secure.yaml index 0659f74..7a337e0 100644 --- a/gremlin-server/conf/gremlin-server-rest-secure.yaml +++ b/gremlin-server/conf/gremlin-server-rest-secure.yaml @@ -66,7 +66,7 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536 authentication: { className: org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator, http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/conf/gremlin-server-secure.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/conf/gremlin-server-secure.yaml b/gremlin-server/conf/gremlin-server-secure.yaml index 721932b..91623b7 100644 --- a/gremlin-server/conf/gremlin-server-secure.yaml +++ b/gremlin-server/conf/gremlin-server-secure.yaml @@ -66,7 +66,7 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536 authentication: { className: org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator, http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/conf/gremlin-server-spark.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/conf/gremlin-server-spark.yaml b/gremlin-server/conf/gremlin-server-spark.yaml index e6510b6..804df73 100644 --- a/gremlin-server/conf/gremlin-server-spark.yaml +++ b/gremlin-server/conf/gremlin-server-spark.yaml @@ -79,7 +79,7 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536 ssl: { enabled: false} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/conf/gremlin-server.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/conf/gremlin-server.yaml b/gremlin-server/conf/gremlin-server.yaml index f310f56..fe11127 100644 --- a/gremlin-server/conf/gremlin-server.yaml +++ b/gremlin-server/conf/gremlin-server.yaml @@ -56,7 +56,7 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536 ssl: { enabled: false} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/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 a5dd0a0..e748fe3 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 @@ -321,11 +321,20 @@ public abstract class AbstractEvalOpProcessor implements OpProcessor { // so iterating next() if the message is not written and flushed would bump the aggregate size beyond // the expected resultIterationBatchSize. Total serialization time for the response remains in // effect so if the client is "slow" it may simply timeout. - if (aggregate.size() < resultIterationBatchSize) aggregate.add(itty.next()); + // + // there is a need to check hasNext() on the iterator because if the channel is not writeable the + // previous pass through the while loop will have next()'d the iterator and if it is "done" then a + // NoSuchElementException will raise its head. + // + // this could be placed inside the isWriteable() portion of the if-then below but it seems better to + // allow iteration to continue into a batch if that is possible rather than just doing nothing at all + // while waiting for the client to catch up + if (aggregate.size() < resultIterationBatchSize && itty.hasNext()) aggregate.add(itty.next()); // send back a page of results if batch size is met or if it's the end of the results being iterated. // also check writeability of the channel to prevent OOME for slow clients. if (ctx.channel().isWritable()) { + if (aggregate.size() == resultIterationBatchSize || !itty.hasNext()) { final ResponseStatusCode code = itty.hasNext() ? ResponseStatusCode.PARTIAL_CONTENT : ResponseStatusCode.SUCCESS; http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml b/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml index 054f0dd..88c95b7 100644 --- a/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml +++ b/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-integration.yaml @@ -55,5 +55,5 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536 http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/bc397eca/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-performance.yaml ---------------------------------------------------------------------- diff --git a/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-performance.yaml b/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-performance.yaml index 841523c..ed7444c 100644 --- a/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-performance.yaml +++ b/gremlin-server/src/test/resources/org/apache/tinkerpop/gremlin/server/gremlin-server-performance.yaml @@ -54,5 +54,5 @@ maxChunkSize: 8192 maxContentLength: 65536 maxAccumulationBufferComponents: 1024 resultIterationBatchSize: 64 -writeBufferHighWaterMark: 32768 +writeBufferLowWaterMark: 32768 writeBufferHighWaterMark: 65536
