Todd Lipcon has posted comments on this change. Change subject: Don't output unobservable rows from the MemRowset ......................................................................
Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/4994/11/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: PS11, Line 63: // This can only be applied to mutation lists that are not in a memory store. can you explain why? PS11, Line 121: if (PREDICT_FALSE( : input_row.redo_head != nullptr : && insertion_timestamp.CompareTo(input_row.redo_head->timestamp()) == 0)) { : // Get the latest mutation : const Mutation* latest = input_row.redo_head; : AdvanceToLastInListConst(&latest); : if (latest->changelist().is_delete() : && insertion_timestamp.CompareTo(latest->timestamp()) == 0) { : iter_->Next(); : continue; : } isn't this specifically only catching the case where the initial insert was deleted in the same transaction? What about a case where we insert at t=1, then delete at t=2, then insert/delete at t=3? that scenario doesn't cause problems at compaction? even after various cases of major GC? -- To view, visit http://gerrit.cloudera.org:8080/4994 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieab52a1aa68494218f91f3acd31ef8ddf352bd57 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes