Adar Dembo has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls

Patch Set 2:

Commit Message:

PS2, Line 9: This simplifies and improves the performance of 
AsyncKuduClient::locateTable by
           : using the client's existing tablet cache.
Nit: there's not enough context in this patch to know why you're making this 
change, especially given what you write below about nobody actually using these 
APIs. I know the answer: it's because it means there's less code to write in 
the non-covering partition patch that follows, but perhaps you can state that 
explicitly as part of the commit description?
File java/kudu-client/src/main/java/org/kududb/client/

Line 1230:   private final class MasterLookupCB implements Callback<Object, 
Master.GetTableLocationsResponsePB> {
Nit: too long now? Unclear.

Line 1876:     private final List<LocatedTablet.Replica> replicas = new 
Would it be safe to use a CopyOnWriteArrayList here, and in doing so avoid the 
need to hold any locks when accessing 'replicas'?

Line 2047:     List<LocatedTablet.Replica> getReplicas() {
Nit: may want to name it "getImmutableReplicas" so that callers know they can't 
modify the list they are handed back.

It's a pleasant convention I remember from my CM days; I don't know whether 
we're really following it in the Java client though.
File java/kudu-client/src/main/java/org/kududb/client/

Line 143:    * Use the {@link KuduScanToken} API instead of this method if 
Why would it not be possible to use the scan token API? Why haven't we 
deprecated this family of methods explicitly?

To view, visit
To unsubscribe, visit

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

Reply via email to