You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/03/08 20:44:32 UTC

[kudu-CR] WIP: java: more error message cleanup

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/6321

to review the following change.

Change subject: WIP: java: more error message cleanup
......................................................................

WIP: java: more error message cleanup

* Negotiator now throws a NonRecoverable KuduException when negotiation
  fails
* Added stack trace truncation to a bunch of places where we know the
  stack traces to be useless (eg when we're throwing an exception in a
  stack called by Netty such as an event handler).
* Changed the Negotiator 'initial response' to be generated at the point
  at which it is picking a SASL mechanism. This way it properly avoids
  picking a mechanism that can't be intialized.
* When the Negotiation fails, fails the outbound RPCs on that connection
  with a status indicating the negotiation failure instead of
  "disconnected".

WIP because I'm not sure that "non-recoverable" exceptions are the right
approach here -- if we have some attempt of a read/write at a tablet
server, and negotiation fails on that tserver, we still want to retry
elsewhere in most cases. This may be too risky for 1.3 (maybe can pull
out some of the safer bits)

Change-Id: I50a254561b431b103caec1eff9f92c9705d78aef
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
5 files changed, 112 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/6321/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I50a254561b431b103caec1eff9f92c9705d78aef
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>

[kudu-CR] WIP: java: more error message cleanup

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: WIP: java: more error message cleanup
......................................................................


Patch Set 1:

(2 comments)

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

Line 304:       LOG.debug("Error receiving a response from: {}", e);
I think you forgot to put hostAndPort back.


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

Line 662:                                final KuduException exception) {
We should only be retrying recoverable exceptions here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50a254561b431b103caec1eff9f92c9705d78aef
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] WIP: java: more error message cleanup

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has abandoned this change. ( http://gerrit.cloudera.org:8080/6321 )

Change subject: WIP: java: more error message cleanup
......................................................................


Abandoned

Similar goals were achieved by later patches like ead756844ce9ada904fcc3666df25692f63e76b8 and ce0db915787b58a79109e6faecc6f1daef9f2850 and cdc73b900608000ff2d2f8bc74c05893338454ba
-- 
To view, visit http://gerrit.cloudera.org:8080/6321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I50a254561b431b103caec1eff9f92c9705d78aef
Gerrit-Change-Number: 6321
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins