Adar Dembo has posted comments on this change.

Change subject: [java client] Redo how we manage exceptions

Patch Set 5:

File java/kudu-client/src/main/java/org/kududb/client/

Line 1029:     Status statusTimedOut = Status.TimedOut(message + request);
> Isn't that "cause" below?
Yes and no. The cause does give the user more information, but if the sequence 
of events is:
  1. Try operation x.
  2. Retry operation x.
  3. Retry operation x.
  n. Timeout.

The timeout itself isn't interesting, and so perhaps even the Status's code 
should reflect the cause. That's what we do in the C++ client: the status code 
contains the actual failure, and the timeout is only added as a prefix of the 
failure's message.
File java/kudu-client/src/main/java/org/kududb/client/

Line 107:       } catch (Exception ex) {
> isAlterTableDone too.
Why? It's an async operation; why would it throw?
File java/kudu-client/src/main/java/org/kududb/client/

PS5, Line 71:       throw new RuntimeException("RowResult block has " + 
bs.length() +
            :           " bytes of data but expected " + expectedSize + " for " 
+ numRows + " rows");
> Pretty sure if this is wrong that it'll go boom below when constructing the
What if we added a factory method to build RowResultIterators, and performed 
the check there? Then we could throw the checked exception from that new method 
and pass it up the call stack?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Jean-Daniel Cryans <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to