Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11859 )
Change subject: (04/05) delta_store: support iteration with is_deleted virtual column ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/common/schema.h@870 PS3, Line 870: size_t > should be int because of Done http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/tablet/delta_store.h File src/kudu/tablet/delta_store.h: http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/tablet/delta_store.h@356 PS3, Line 356: size_t > should be int Done http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/tablet/delta_store.cc File src/kudu/tablet/delta_store.cc: http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/tablet/delta_store.cc@249 PS3, Line 249: uint32_t idx_in_block = row_id - prev_prepared_idx_; > is this guaranteed to be disjoint from rows in deleted_ ? If so I don't see Yes, it's guaranteed to be disjoint, and yes, we do need to set both deleted rows to true and reinserted ones to false. To understand why, consider how the DeltaIteratorMerger works. It's a vector of DeltaIterators and when we make our usual PrepareBlock()/ApplyUpdates() calls, the merger will fan them out to every DeltaIterator in order. As such, it's possible for the first iterator in the merger to delete row foo and the next iterator to reinsert row foo, which means it's important for rows that we think are reinserted to be explicitly unset _back_ to false. That's the same reasoning behind the selection vector logic in ApplyDeletes. There's a comment in UpdateDeletionState that sort of hints at this; I'll try to make it more explicit. http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/tablet/memrowset.h File src/kudu/tablet/memrowset.h: http://gerrit.cloudera.org:8080/#/c/11859/3/src/kudu/tablet/memrowset.h@598 PS3, Line 598: size_t > int here also I think Done -- To view, visit http://gerrit.cloudera.org:8080/11859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibafcae9f9f82a6c2d9a8afedb8fbf07d6e3069ec Gerrit-Change-Number: 11859 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 21 Nov 2018 23:35:38 +0000 Gerrit-HasComments: Yes
