You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ok...@apache.org on 2016/05/10 13:33:50 UTC
[11/13] incubator-tinkerpop git commit: Expanded the number of
exceptions that the driver would "replace" a connection on.
Expanded the number of exceptions that the driver would "replace" a connection on.
By opting to replace, it avoids the expense of marking a host as dead when it really isn't. CTR
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/690db2f8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/690db2f8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/690db2f8
Branch: refs/heads/TINKERPOP-1293
Commit: 690db2f82162d2e4bb2d22733f586cf010b5be37
Parents: c8901f3
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Mon May 9 16:39:21 2016 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Mon May 9 16:39:21 2016 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../tinkerpop/gremlin/driver/Connection.java | 20 +++++++++++--------
.../server/GremlinDriverIntegrateTest.java | 21 ++++++++++++++++++++
3 files changed, 34 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/690db2f8/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 41e74be..1163447 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/incubator-tinkerpop/master/docs/
TinkerPop 3.1.3 (NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Fixed bug in `gremlin-driver` where certain channel-level errors would not allow the driver to reconnect.
* Bumped SLF4J to 1.7.21 as previous versions suffered from a memory leak.
* Fixed a bug in `Neo4jGraphStepStrategy` where it wasn't defined properly as a `ProviderOptimizationStrategy`.
* Renamed `AndTest.get_g_V_andXhasXage_gt_27X__outE_count_gt_2X_name` to `get_g_V_andXhasXage_gt_27X__outE_count_gte_2X_name` to match the traversal being tested.
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/690db2f8/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 a5dd7d5..cecfbc5 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
@@ -18,6 +18,8 @@
*/
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;
@@ -202,15 +204,17 @@ final class Connection {
// so this isn't going to be like a dead host situation which is handled above on a failed
// write operation.
//
- // in the event of an IOException, that will typically mean that the Connection might have
- // been closed from the server side. this is typical in situations like when a request is
- // sent that exceeds maxContentLength (the server closes the channel on its side). if the
- // Connection is simply returned to the pool then it will be used again on a future request
- // and the server will refuse it and make it appear as a dead host as the write will not
- // succeed. instead, the Connection gets replaced which destroys the dead channel on the
- // client and allows a new one to be reconstructed.
+ // in the event of an IOException (typically means that the Connection might have
+ // been closed from the server side - this is typical in situations like when a request is
+ // sent that exceeds maxContentLength and the server closes the channel on its side) or other
+ // exceptions that indicate a non-recoverable state for the Connection object
+ // (a netty CorruptedFrameException is a good example of that), the Connection cannot simply
+ // be returned to the pool as future uses will end with refusal from the server and make it
+ // appear as a dead host as the write will not succeed. instead, the Connection needs to be
+ // replaced in these scenarios which destroys the dead channel on the client and allows a new
+ // one to be reconstructed.
readCompleted.exceptionally(t -> {
- if (t instanceof IOException) {
+ if (t instanceof IOException || t instanceof CodecException) {
if (pool != null) pool.replaceConnection(thisConnection);
} else {
thisConnection.returnToPool();
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/690db2f8/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 4f6a655..8515e8a 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
@@ -130,6 +130,27 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
}
@Test
+ public void shouldEventuallySucceedAfterChannelLevelError() throws Exception {
+ final Cluster cluster = Cluster.build().addContactPoint("localhost")
+ .reconnectIntialDelay(500)
+ .reconnectInterval(500)
+ .maxContentLength(1024).create();
+ final Client client = cluster.connect();
+
+ try {
+ client.submit("def x = '';(0..<1024).each{x = x + '$it'};x").all().get();
+ fail("Request should have failed because it exceeded the max content length allowed");
+ } catch (Exception ex) {
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertThat(root.getMessage(), containsString("Max frame length of 1024 has been exceeded."));
+ }
+
+ assertEquals(2, client.submit("1+1").all().join().get(0).getInt());
+
+ cluster.close();
+ }
+
+ @Test
public void shouldEventuallySucceedAfterMuchFailure() throws Exception {
final Cluster cluster = Cluster.build().addContactPoint("localhost").create();
final Client client = cluster.connect();