Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12205 )
Change subject: WIP: KUDU-2645. tablet: Add ghost row de-duplication support to MergeIterator ...................................................................... Patch Set 1: (2 comments) Just skimmed this. http://gerrit.cloudera.org:8080/#/c/12205/1/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/12205/1/src/kudu/common/generic_iterators.cc@350 PS1, Line 350: const int kIsDeletedColIndex = schema_.find_first_is_deleted_virtual_column(); This can be hoisted out of the outer loop, and maybe used to control whether the ghost sifting even happens in the first place? http://gerrit.cloudera.org:8080/#/c/12205/1/src/kudu/common/generic_iterators.cc@355 PS1, Line 355: RowBlockRow dst_row = dst->row(dst_row_idx++); This can overflow the RowBlock. We need to avoid doing that, but also make sure we don't "consume" a row as part of our "find all rows sharing the same smallest row key" and not return it to the client. -- To view, visit http://gerrit.cloudera.org:8080/12205 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00614b3fa5c6b4e7b620bb78489e24c5ad44daee Gerrit-Change-Number: 12205 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Comment-Date: Wed, 09 Jan 2019 22:48:57 +0000 Gerrit-HasComments: Yes