Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10983 )
Change subject: KUDU-1291. Efficiently support predicates on non-prefix key components ...................................................................... Patch Set 14: (3 comments) http://gerrit.cloudera.org:8080/#/c/10983/12/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/10983/12/src/kudu/tablet/cfile_set.cc@551 PS12, Line 551: atus CFileSet::Iterator::SeekToNextPrefixKey(size_t num_prefix_cols, bool cache_seeked_value) { : gscoped_ptr<EncodedKey> enc_key; : Arena arena(kEncodedCompositeKeyMaxSize); > Might be helpful: from util/memory/arena.h and the definition of Reset(): +1 Using dedicated Arena per iterator instance sounds good to me. It seems we can easily reset it in the long loop of the SkipToNextScan() method. http://gerrit.cloudera.org:8080/#/c/10983/14/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/10983/14/src/kudu/tablet/cfile_set.cc@770 PS14, Line 770: Check to see whether we have effectively seeked backwards > We effectively have two pointers we are tracking with skip-scan: the primar Adding a comment would definitely help. That might be a comment for the method, as well as an extra for already existing high-level description of the algorithm at line 656. http://gerrit.cloudera.org:8080/#/c/10983/15/src/kudu/tablet/cfile_set.cc File src/kudu/tablet/cfile_set.cc: http://gerrit.cloudera.org:8080/#/c/10983/15/src/kudu/tablet/cfile_set.cc@557 PS15, Line 557: // columns of the cached value obtained after > I think it's possible to pass an instance of arena from the upper level to And the best approach I think is suggested by Andrew -- have an instance of Arena as a member of CFileSet::Iterator. Probably, it's necessary to reset it after use in every cycle of the loop in SkipToNextScan() method. -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 14 Gerrit-Owner: Anupama Gupta <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Anupama Gupta <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 16 Aug 2018 07:24:09 +0000 Gerrit-HasComments: Yes
