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 2019/06/18 16:36:08 UTC

[tinkerpop] branch TINKERPOP-2248 created (now a0d6923)

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a change to branch TINKERPOP-2248
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.


      at a0d6923  TINKERPOP-2248 Force replacement of connections on certain errors

This branch includes the following new commits:

     new a0d6923  TINKERPOP-2248 Force replacement of connections on certain errors

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[tinkerpop] 01/01: TINKERPOP-2248 Force replacement of connections on certain errors

Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch TINKERPOP-2248
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit a0d6923e2f3dfdf981c046341f031fd87ad4ac65
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Tue Jun 18 12:31:48 2019 -0400

    TINKERPOP-2248 Force replacement of connections on certain errors
    
    Relying only on the Connection.isDead() check to determine connection replacement seemed to introduce a regression as of 2cd84ff1f3944d67d44cbba5bb032a1c57377975 The shouldBlockRequestWhenTooBig was failing somewhat randomly since that change. Not sure if there is a better way to make isDead() be the only check needed for connection replacement in this context, but this change brings the logic back that checks exception types that were forcing replacement prior to thise change.
---
 CHANGELOG.asciidoc                                                | 1 +
 .../main/java/org/apache/tinkerpop/gremlin/driver/Connection.java | 5 ++++-
 .../java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java  | 8 ++++----
 .../main/java/org/apache/tinkerpop/gremlin/driver/Handler.java    | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index e60b668..01dc695 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -29,6 +29,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Improved exception and messaging for gt/gte/lt/lte when one of the object isn't a `Comparable`.
 * Added test infrastructure to check for storage iterator leak.
 * Fixed multiple iterator leaks in query processor.
+* Forced replacement of connections in Java driver for certain exception types that seem to ultimately kill the connection.
 * Changed the `reverse()` of `desc` and `asc` on `Order` to not use the deprecated `decr` and `incr`.
 * Fixed bug in `MatchStep` where the correct was not properly determined.
 * Fixed bug where client/server exception mismatch when server throw StackOverflowError
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 bc91b3c..360c844 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,7 @@
  */
 package org.apache.tinkerpop.gremlin.driver;
 
+import io.netty.handler.codec.CodecException;
 import org.apache.tinkerpop.gremlin.driver.exception.ConnectionException;
 import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
 import io.netty.bootstrap.Bootstrap;
@@ -26,6 +27,8 @@ import io.netty.channel.ChannelPromise;
 import io.netty.channel.socket.nio.NioSocketChannel;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
 import java.net.URI;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
@@ -283,7 +286,7 @@ final class Connection {
      * on the client and allows a new one to be reconstructed.
      */
     private void handleConnectionCleanupOnError(final Connection thisConnection, final Throwable t) {
-        if (thisConnection.isDead()) {
+        if (thisConnection.isDead() || t instanceof IOException || t instanceof CodecException) {
             if (pool != null) pool.replaceConnection(thisConnection);
         } else {
             thisConnection.returnToPool();
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 a6b09f0..332731e 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
@@ -403,7 +403,7 @@ final class ConnectionPool {
      * as part of a schedule in {@link Host} to periodically try to create working connections.
      */
     private boolean tryReconnect(final Host h) {
-        logger.debug("Trying to re-establish connection on {}", host);
+        logger.debug("Trying to re-establish connection on {}", h);
 
         Connection connection = null;
         try {
@@ -414,10 +414,10 @@ final class ConnectionPool {
             f.get().all().get();
 
             // host is reconnected and a connection is now available
-            this.cluster.loadBalancingStrategy().onAvailable(host);
+            this.cluster.loadBalancingStrategy().onAvailable(h);
             return true;
         } catch (Exception ex) {
-            logger.debug("Failed reconnect attempt on {}", host);
+            logger.debug("Failed reconnect attempt on {}", h);
             if (connection != null) definitelyDestroyConnection(connection);
             return false;
         }
@@ -441,7 +441,7 @@ final class ConnectionPool {
         int minInFlight = Integer.MAX_VALUE;
         Connection leastBusy = null;
         for (Connection connection : connections) {
-            int inFlight = connection.borrowed.get();
+            final int inFlight = connection.borrowed.get();
             if (!connection.isDead() && inFlight < minInFlight) {
                 minInFlight = inFlight;
                 leastBusy = connection;
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
index b333287..7b12890 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
@@ -204,7 +204,7 @@ final class Handler {
 
         @Override
         public void channelInactive(final ChannelHandlerContext ctx) throws Exception {
-            // occurs when the server shutsdown in a disorderly fashion, otherwise in an orderly shutdown the server
+            // occurs when the server shuts down in a disorderly fashion, otherwise in an orderly shutdown the server
             // should fire off a close message which will properly release the driver.
             super.channelInactive(ctx);