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