Adar Dembo has posted comments on this change.

Change subject: log block manager: detect and repair unpunched holes
......................................................................


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6717/7/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:

Line 860:   // when it fires.
> Since slop isn't a part of the gflag, it may be clearer to update the "slop
Sure, will use your proposed rewording verbatim.


http://gerrit.cloudera.org:8080/#/c/6717/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

PS5, Line 79:  0.1
> thanks for the explanation. it would be good to have this findings written 
Most of it is documented on L1815. I'll add a little more, but I'd rather not 
document raw performance numbers (percentages and raw times) since that may 
fluctuate as the code evolves.


http://gerrit.cloudera.org:8080/#/c/6717/7/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

PS7, Line 81:  
> nit: is
Done


PS7, Line 1830: slop in our size measurements. Too little
              :       // slop and we'll do unnecessary work at startup
> nit: Make the connection between "slop" and FLAGS_log_container_excess_spac
Done


PS7, Line 1841: measured_size
> nit: This is more of a threshold, rather than a measured size. Could we ren
Done


PS7, Line 2061: // Clearing this vector drops the last references to the 
LogBlocks within,
              :   // triggering the repunching operations.
> Is this enforced by the fact that we're calling Repair() with std::move(nee
I think that's too implementation-specific.

In the worst case and the std::move() is accidentally removed, the last 
references will be dropped when OpenDataDir() goes out of scope. From the 
perspective of the block manager user, it's effectively the same.


-- 
To view, visit http://gerrit.cloudera.org:8080/6717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I016ea401380f4c8c7b1fd907ff67cb595f377dd1
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to