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: (5 comments) 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@1873 PS2, Line 1873: MetricRegistry registry; : scoped_refptr<MetricEntity> entity = METRIC_ENTITY_server.Instantiate(®istry, "test"); : ASSERT_OK(ReopenBlockManager(entity)); > If you're not testing metrics, I don't think you need to do this. Done http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager-test.cc@1877 PS2, Line 1877: // Create a bunch of blocks. > Nit: new code should use BlockCreationTransactions to create blocks. Done http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager-test.cc@1895 PS2, Line 1895: : // Lower the max container size, now the container is full. : FLAGS_log_container_max_size = 1; > Doing this here feels a little weird. It's not something you could do in pr Done http://gerrit.cloudera.org:8080/#/c/12308/2/src/kudu/fs/log_block_manager-test.cc@1920 PS2, Line 1920: CHECK_OK(bm_->OpenBlock(block_id, &block)); > ASSERT_OK. 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@554 PS2, Line 554: // Tries to mark the container as 'dead'. It can only be set successfully : // once, and it is deleted when the container goes out of scope. > Nit: "Tries to mark the container as 'dead', which means it will be deleted 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: 2 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Fri, 01 Feb 2019 01:36:11 +0000 Gerrit-HasComments: Yes
