Hao Hao 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@12
PS2, Line 12: That's because
Is it possible to detect the flakiness of block_manager-stress-test by looping 
the test by dist-test? If so, could you verify the test is no longer flaky 
after your fix and add this information in the commit message?


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@1877
PS2, Line 1877:   // Create a bunch of blocks.
> Nit: new code should use BlockCreationTransactions to create blocks.
+1.


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 
transaction happening concurrently?


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 block 
is not closed? Since if the block is not closed, the live block count is not 
incremented yet. There could be a chance that the container is deleted before 
DoCloseBlocks() being called in Abort().



--
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: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <[email protected]>
Gerrit-Comment-Date: Fri, 01 Feb 2019 01:43:47 +0000
Gerrit-HasComments: Yes

Reply via email to