Mike Percy has posted comments on this change. Change subject: KUDU-237 (part 2) - Add support for REINSERT in delta files ......................................................................
Patch Set 12: (12 comments) http://gerrit.cloudera.org:8080/#/c/4995/12/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: Line 299: CHECK_NE(left_latest->changelist().is_delete(), right_latest->changelist().is_delete()); nit: maybe more intuitive to write this as CHECK(!(left_latest->changelist().is_delete() && right_latest->changelist().is_delete())); PS12, Line 708: if typo: is Line 969: // When we see a reinsert REDO we do the following: nit: indentation seems off on this comment block. Line 991: dst_row, static_cast<Arena*>(nullptr), &undo_encoder), ERROR, I know you didn't write this but is it right not to use an arena here? http://gerrit.cloudera.org:8080/#/c/4995/12/src/kudu/tablet/compaction.h File src/kudu/tablet/compaction.h: PS12, Line 167: multiple versions > I think the term 'version' here is a bit confusing, because even without th or "when a row appears in multiple RowSets" http://gerrit.cloudera.org:8080/#/c/4995/12/src/kudu/tablet/deltafile.cc File src/kudu/tablet/deltafile.cc: Line 687: // Visitor which applies deletes to the selection vector. > we should rename this visitor to LivenessVisitor or DeleteReinsertVisitor o +1 for LivenessVisitor http://gerrit.cloudera.org:8080/#/c/4995/12/src/kudu/tablet/tablet-test.cc File src/kudu/tablet/tablet-test.cc: PS12, Line 228: tow typo PS12, Line 229: all s/all/full/ Line 237: // Insert one row punctuation here and below in the comments Line 238: CHECK_OK(this->InsertTestRow(&writer, 1, 0)); ASSERT_OK() ? Line 239: ASSERT_FALSE(writer.last_op_result().has_failed_status()); No need to check last_op_result(), InsertTestRow() already returns non-OK via LocalTabletWriter::WriteBatch() if any of the ops failed. Also superfluous below for DeleteTestRow(). PS12, Line 255: forth typo: fourth -- To view, visit http://gerrit.cloudera.org:8080/4995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1173b2bea721b376f2b6049be20f57307582c47 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
