You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/09/07 02:15:54 UTC

[2/2] kudu git commit: KUDU-2130 (part 2): more fixes for ITClientStress

KUDU-2130 (part 2): more fixes for ITClientStress

This fixes some more race conditions in connection termination in the
same vein as part 1.  It also filters benign SSLException from being
returned back to callers.

Change-Id: Ic3f518513931c660fd93b4272d1b1fceb268f191
Reviewed-on: http://gerrit.cloudera.org:8080/7964
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit f41a5c2beeee3afc8b4703c8047b347490b24c19)
Reviewed-on: http://gerrit.cloudera.org:8080/7979


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

Branch: refs/heads/branch-1.5.x
Commit: b31f3e5cc837dcd42f43ec90f0201480e15486a7
Parents: 89677d8
Author: Dan Burkert <da...@apache.org>
Authored: Tue Sep 5 15:39:36 2017 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Thu Sep 7 02:10:23 2017 +0000

----------------------------------------------------------------------
 .../java/org/apache/kudu/client/Connection.java | 28 ++++++++++++++------
 .../org/apache/kudu/client/ITClientStress.java  |  2 ++
 2 files changed, 22 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/b31f3e5c/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
index 8b1febb..96395e4 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
@@ -214,6 +214,9 @@ class Connection extends SimpleChannelUpstreamHandler {
                                final ChannelStateEvent e) {
     lock.lock();
     try {
+      if (state == State.TERMINATED) {
+        return;
+      }
       Preconditions.checkState(state == State.CONNECTING);
       state = State.NEGOTIATING;
     } finally {
@@ -313,6 +316,9 @@ class Connection extends SimpleChannelUpstreamHandler {
     if (m instanceof Negotiator.Failure) {
       lock.lock();
       try {
+        if (state == State.TERMINATED) {
+          return;
+        }
         Preconditions.checkState(state == State.NEGOTIATING);
         Preconditions.checkState(inflightMessages.isEmpty());
 
@@ -347,6 +353,9 @@ class Connection extends SimpleChannelUpstreamHandler {
     Callback<Void, CallResponseInfo> responseCbk;
     lock.lock();
     try {
+      if (state == State.TERMINATED) {
+        return;
+      }
       Preconditions.checkState(state == State.READY);
       responseCbk = inflightMessages.remove(callId);
     } finally {
@@ -408,20 +417,23 @@ class Connection extends SimpleChannelUpstreamHandler {
           getLogPrefix());
       error = new RecoverableException(Status.NetworkError(message), e);
       LOG.info(message, e);
-    } else {
-      String message = String.format("%s unexpected exception from downstream on %s",
-                                     getLogPrefix(), c);
-      error = new RecoverableException(Status.NetworkError(message), e);
-
+    } else if (e instanceof SSLException && explicitlyDisconnected) {
       // There's a race in Netty where, when we call Channel.close(), it tries
       // to send a TLS 'shutdown' message and enters a shutdown state. If another
       // thread races to send actual data on the channel, then Netty will get a
       // bit confused that we are trying to send data and misinterpret it as a
       // renegotiation attempt, and throw an SSLException. So, we just ignore any
       // SSLException if we've already attempted to close, otherwise log the error.
-      if (!(e instanceof SSLException) || !explicitlyDisconnected) {
-        LOG.error(message, e);
-      }
+      error = new RecoverableException(Status.NetworkError(
+          String.format("%s disconnected from peer", getLogPrefix())));
+    } else {
+      // If the connection was explicitly disconnected via a call to disconnect(), we should
+      // have either gotten a ClosedChannelException or an SSLException.
+      assert !explicitlyDisconnected;
+      String message = String.format("%s unexpected exception from downstream on %s",
+                                     getLogPrefix(), c);
+      error = new RecoverableException(Status.NetworkError(message), e);
+      LOG.error(message, e);
     }
 
     cleanup(error);

http://git-wip-us.apache.org/repos/asf/kudu/blob/b31f3e5c/java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java b/java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
index a3632c9..71066b7 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
@@ -80,6 +80,8 @@ public class ITClientStress extends BaseKuduTest {
         cla.getAppendedText().contains("NullPointerException"));
     assertFalse("log contained SSLException",
         cla.getAppendedText().contains("SSLException"));
+    assertFalse("log contained IllegalStateException",
+        cla.getAppendedText().contains("IllegalStateException"));
   }
 
   /**