Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8441 )
Change subject: disk failure: pass Statuses instead of failing CHECKs along the Flush DMS path ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8441/1/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/8441/1/src/kudu/tablet/delta_tracker.cc@244 PS1, Line 244: size_t i = 0; i < list.size() - 1; i++ It's not introduced in this changelist, but I suspect it would it be an issue if list.size() == 0. Consider rewriting with for (size_t i = 1; i < list.size(); i++) { RETURN_NOT_OK_PREPEND(ValidateDeltaOrder(list[i - 1], list[i], type), "Failed to validate delta order"); } http://gerrit.cloudera.org:8080/#/c/8441/1/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/8441/1/src/kudu/tablet/diskrowset.cc@560 PS1, Line 560: MajorCompactDeltaStoresWithColumnIds I tried to track this up to the call chains and in the chains [Major|Minor]DeltaCompactionOp::Perform() Tablet::CompactWorstDeltas() DiskRowSet::MajorCompactDeltaStoresWithColumnIds() expecting to see CHECK_OK() if compaction->UpdateDeltaTracker(delta_tracker_.get()) at line 590 returned non-OK status, but I didn't find it. Did I miss something here? -- To view, visit http://gerrit.cloudera.org:8080/8441 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0241cd4206ce77ef1fb334458b091bc2092f4141 Gerrit-Change-Number: 8441 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 02 Nov 2017 04:41:10 +0000 Gerrit-HasComments: Yes
