kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [1/2] kudu git commit: KUDU-2130: java client: handle termination during negotiation edge case
Date Thu, 07 Sep 2017 02:15:53 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.5.x 9101f85fa -> b31f3e5cc


KUDU-2130: java client: handle termination during negotiation edge case

There was an edge case where a Connection can be terminated while negotiation
is completing. This would result in a scary looking log message:

  18:24:07.776 [DEBUG - New I/O worker #8112] (Connection.java:649) [peer master-127.32.133.1:64032]
cleaning up while in state NEGOTIATING due to: connection disconnected
  18:24:07.781 [ERROR - New I/O worker #8112] (Connection.java:418) [peer master-127.32.133.1:64032]
unexpected exception from downstream on [id: 0xdd52bacc, /127.0.0.1:55318 :> /127.32.133.1:64032]
  java.lang.NullPointerException
     at org.apache.kudu.client.Connection.messageReceived(Connection.java:271)
      at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
      at org.apache.kudu.client.Connection.handleUpstream(Connection.java:236)
      at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
      at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)

but in reality the error message is harmless; it just indicates that the
connection has been terminated while the connection's messageReceived handler
is clearing the message queue. This interruption is possible because of
82a8e9f99, which fixed a deadlock in Connection. This commit recognizes when
this race has occured, and early exits from messageReceived.

Change-Id: I3e9d4a6535ae82973743e4ac1071de0aac92b08b
Reviewed-on: http://gerrit.cloudera.org:8080/7960
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit f0aa3b3f194146760597e6ab88c304c6f408073c)
Reviewed-on: http://gerrit.cloudera.org:8080/7978


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

Branch: refs/heads/branch-1.5.x
Commit: 89677d83c8bb3a1697fe95560b3b71634ee94dd4
Parents: 9101f85
Author: Dan Burkert <danburkert@apache.org>
Authored: Tue Sep 5 13:39:06 2017 -0700
Committer: Dan Burkert <danburkert@apache.org>
Committed: Thu Sep 7 02:10:18 2017 +0000

----------------------------------------------------------------------
 .../main/java/org/apache/kudu/client/Connection.java   | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/89677d83/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 f21d2bb..8b1febb 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
@@ -261,14 +261,14 @@ class Connection extends SimpleChannelUpstreamHandler {
     if (m instanceof Negotiator.Success) {
       lock.lock();
       try {
-        Preconditions.checkState(state == State.NEGOTIATING);
-        Preconditions.checkState(inflightMessages.isEmpty());
         negotiationResult = (Negotiator.Success) m;
+        Preconditions.checkState(state == State.TERMINATED || inflightMessages.isEmpty());
 
         // Before switching to the READY state, it's necessary to empty the queuedMessages.
There
         // might be concurrent activity on adding new messages into the queue if enqueueMessage()
         // is called in the middle.
-        while (!queuedMessages.isEmpty()) {
+        while (state != State.TERMINATED && !queuedMessages.isEmpty()) {
+
           // Register the messages into the inflightMessages before sending them to the wire.
This
           // is to be able to invoke appropriate callback when the response received. This
should
           // be done under the lock since the inflightMessages itself does not provide any
@@ -292,8 +292,13 @@ class Connection extends SimpleChannelUpstreamHandler {
             lock.lock();
           }
         }
+        // The connection may have been terminated while the lock was dropped.
+        if (state == State.TERMINATED) {
+          return;
+        }
+
+        Preconditions.checkState(state == State.NEGOTIATING);
 
-        assert queuedMessages.isEmpty();
         queuedMessages = null;
         // Set the state to READY -- that means the incoming messages should be no longer
put into
         // the queuedMessages, but sent to wire right away (see the enqueueMessage() for
details).


Mime
View raw message