Dan Burkert has posted comments on this change.

Change subject: KUDU-1811: C++ client: use larger batches when fetching scan 
tokens
......................................................................


Patch Set 5:

(2 comments)

Overall I think this is a good change, but I wonder if it wouldn't just be 
simpler to change the existing constant to 100 and call it good enough.  10 
does seem like a low number in any situation given the overhead of RPCs.

http://gerrit.cloudera.org:8080/#/c/7748/5/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

Line 812:                                         int requested_batch_size) {
Might be good to name this 'max_returned_locations' to keep it consistent.


http://gerrit.cloudera.org:8080/#/c/7748/5/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

Line 390:   void LookupTabletsByKeyOrNext(const KuduTable* table,
Instead of creating a new method, it may be cleaner to add an optional argument 
to the existing 'LookupTabletByKeyOrNext' method, with the default value of 
kFetchTabletsPerPointLookup.  Then you could expose kFetchTabletsPerRangeLookup 
as a public constant, and use it from the scan token generator.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I79340dc9963944454770d82a2fbaba1b0c8a810e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jun He <junhe...@gmail.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Jun He <junhe...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to