Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12197 )
Change subject: generic_iterators: basic MergeIterator dominance ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/12197/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12197/5//COMMIT_MSG@11 PS5, Line 11: omit sub-iterator A if its current block's first primary key is : lexicographically greater than another sub-iterator B's current block > is this phrased backwards? ie: Whoops, yes. http://gerrit.cloudera.org:8080/#/c/12197/5/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/12197/5/src/kudu/common/generic_iterators.cc@536 PS5, Line 536: size_t available = 0; > is it worth adding: Sounds reasonable. http://gerrit.cloudera.org:8080/#/c/12197/5/src/kudu/common/generic_iterators.cc@593 PS5, Line 593: [](MergeIterState* s) { delete s; }); > if the dominated iterators were stored as a min-heap by their min_key, this As we discussed offline, I'm going to evaluate this as part of a follow-up patch, so I can see what kind of effect it has in isolation. http://gerrit.cloudera.org:8080/#/c/12197/5/src/kudu/common/generic_iterators.cc@606 PS5, Line 606: // 2. However, pulling a new block can't cause 'smallest' to dominate any > similarly here if we have a heap of states_ we could O(1) find the state_ w See above. -- To view, visit http://gerrit.cloudera.org:8080/12197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If59d831240af15bfa7ef5709ec3d105d13b28322 Gerrit-Change-Number: 12197 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 14 Jan 2019 21:03:58 +0000 Gerrit-HasComments: Yes
