Jun He has posted comments on this change. Change subject: KUDU-1811: C++ client: use larger batches when fetching scan tokens ......................................................................
Patch Set 5: (4 comments) I think some scale tests are needed to check the impact of the change (from 10 to 100). Additionally, it may be still valid to have different settings for different sets of operations. No matter which way we decide to go, I think we need to make Java and C++ client to share the similar logic and be consistent. Please let me know your comment. Thanks. http://gerrit.cloudera.org:8080/#/c/7748/5/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: Line 76: /** > style nit: we use // even for block comments Done Line 812: int requested_batch_size) { > Might be good to name this 'max_returned_locations' to keep it consistent. Done Line 1048: string partition_key, > nit: alignment Done 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 argu Thanks for the comments. I thought about it but went this way to make the C++ client implementation to be consistent with Java client. Also, it followed the current design to hide the kMaxReturnedTableLocations constant using wrapper methods (LookupTabletByKey and LookupTabletByKeyOrNext in C++ and getTabletsLocations in Java) from token generator. Please let me know your comment. Thanks. -- 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
