Dan Burkert has posted comments on this change.

Change subject: KUDU-1657: read-only FsManager::Open on active tablet can crash
......................................................................


Patch Set 2:

(3 comments)

Test added in slow mode.

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

Line 496:     if (data_file_size < record.offset() + record.length()) {
> PREDICT_FALSE to annotate that this is a rare race
Done


PS2, Line 499: in order to account for the latest appends
> Good find Dan, this is not a review but a curiosity Qn: Looking at the desc
Sort of; the log block manager is careful to only write to the metadata file 
after writing the data file.  So if the other thread or process that's reading 
the metadata file in read-only mode sees an entry for a block in the metadata 
file, it should be guaranteed that if it then rereads the data file length, the 
length will be past the end of the block.


PS2, Line 504: local_records.back()
> Nit: replace with record ?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If68b04f1f1b8cd099120a220b1245ecf8f422770
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to