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: (2 comments) http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/cfile/cfile_reader.h File src/kudu/cfile/cfile_reader.h: http://gerrit.cloudera.org:8080/#/c/10983/10/src/kudu/cfile/cfile_reader.h@59 PS10, Line 59: static const int kEncodedCompositeKeyMaxSize = > Can we test what happens when someone violates this? Yep, it would be nice to have a test with super-wide primary key (multiple columns, big cells) and try to run scans with skip scan enabled. As for the desired behavior, at least tservers should not crash, and the logic of the skip scan optimizations should behave predictably, not outright failing the whole scan operation. 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@699 PS14, Line 699: use_skip_scan_ = false; : return Status::OK(); I think it makes sense to introduce a metric to capture cases when skip scan decided to bail due to the seek cutoff condition. For examples, please looks for examples in the code: check for METRIC_DEFINE_gauge_int64 and INSTANTIATE_METRIC patterns. -- 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 <anupama.gu...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Anupama Gupta <anupama.gu...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 14 Aug 2018 20:18:45 +0000 Gerrit-HasComments: Yes