Alexey Serbin has posted comments on this change.

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


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7964/1/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java:

Line 216:     try {
Would it make sense to add the short-circuiting

      if (state == State.TERMINATED) {
        return;
      }

check here as well?  Or we have some guarantees from Netty that it would not be 
the case?


Line 354:         return;
I'm curious -- did it happen in the wild that the message arrived in here but 
the connection has been terminated already?


PS1, Line 427: assert !explicitlyDisconnected;
I'm not sure it's good to have this assertion here -- this case covers not only 
SSLException case, as I can see.

Could you add a comment with reasoning behind adding this assertion if you 
think it's worth adding this assertion?


-- 
To view, visit http://gerrit.cloudera.org:8080/7964
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3f518513931c660fd93b4272d1b1fceb268f191
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to