Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Fix GetTableLocations error handling
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3295/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 764:    * "Errback" used to delayed-retry a RPC if it fails due to no 
leader master being found.
Javadoc needs to be modified.


Line 793:         request.errback(arg);
I was surprised originally to see this issue, but it makes sense. The errback 
in this case is added on the GetTableLocationsResponse, not the actual RPC. I 
wonder if we have other cases like this where we assume we are sending the 
errback all the way to the client., but in reality they get lost.


http://gerrit.cloudera.org:8080/#/c/3295/2/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java:

PS2, Line 113: RPC get stuck.
nit: to* get stuck.


Line 119:     session.setFlushMode(AsyncKuduSession.FlushMode.AUTO_FLUSH_SYNC);
Isn't that line unnecessary?


Line 126:       Thread.sleep(2000);
Can we find another way to find when the tablet is deleted to not sleep for 2 
seconds?


PS2, Line 131: cause table is deleted.
nit: because the table was deleted.


Line 132:         e.printStackTrace();
Why print it if it's expected?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <decst...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to