helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12308 )
Change subject: KUDU-2665: LBM may delete containers with live blocks ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/12308/3/src/kudu/fs/log_block_manager-test.cc File src/kudu/fs/log_block_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/12308/3/src/kudu/fs/log_block_manager-test.cc@1878 PS3, Line 1878: // The size of the container will be 10*4KB after alignment. > This may not necessarily be true; the filesystem block size may vary from f Done http://gerrit.cloudera.org:8080/#/c/12308/3/src/kudu/fs/log_block_manager-test.cc@1916 PS3, Line 1916: if (close_block) { > The idea here is to test Abort(), right? If so, could you note that in the Done http://gerrit.cloudera.org:8080/#/c/12308/3/src/kudu/fs/log_block_manager-test.cc@1928 PS3, Line 1928: ASSERT_EQ(Status::NotFound("").CodeAsString(), s.CodeAsString()); > Simpler to do: Done http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager.cc@1549 PS2, Line 1549: container_->blocks_being_written_incr(-1); > Sounds good, would you mind adding a comment here to explain it? Done -- To view, visit http://gerrit.cloudera.org:8080/12308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I894f32b1c164ae7770c92171850edd167dfaf8ad Gerrit-Change-Number: 12308 Gerrit-PatchSet: 3 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Sat, 02 Feb 2019 00:04:49 +0000 Gerrit-HasComments: Yes
