helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12308 )
Change subject: KUDU2665: LBM may delete containers with live blocks ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/12308/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12308/2//COMMIT_MSG@7 PS2, Line 7: KUDU2665: LBM may delete containers with live blocks > Nit: add a dash between 'KUDU' and '2665'. Done http://gerrit.cloudera.org:8080/#/c/12308/2//COMMIT_MSG@12 PS2, Line 12: That's because > Is it possible to detect the flakiness of block_manager-stress-test by loop sorry, i have no idea. what does 'dist-test' mean? http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager-test.cc File src/kudu/fs/log_block_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager-test.cc@1914 PS2, Line 1914: ASSERT_OK(writer->Close()); > Maybe add a test when a block is never closed, and have a block deletion tr 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); > We should probably decrement 'blocks_being_written' after checking if the b yep, that's a very good point. but i think the order here can help to avoid unnecessary hole punch, and the container should not be deleted because we have the reference to the container which is 'container_'. -- 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: 2 Gerrit-Owner: helifu <hzhel...@corp.netease.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu <hzhel...@corp.netease.com> Gerrit-Comment-Date: Fri, 01 Feb 2019 03:44:07 +0000 Gerrit-HasComments: Yes
