Adar Dembo has posted comments on this change.

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


Patch Set 1:

(2 comments)

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

PS1, Line 498:       // This codepath can be used in read-only mode as part of 
a tool running
             :       // against a live tablet server. In this case, the tablet 
server may be
             :       // actively appending entries to the container, so the 
data_file_size may
             :       // need to be occasionally updated to account for new data 
entries.
             :       // CheckBlockRecord will crash the process when 
data_file_size is less
             :       // than what the metadata record indicates.
Nit: can you reword this comment to reduce its scope? We're deep inside a block 
manager so we shouldn't need to mention tablet servers, tools, etc. Something 
terse like "It's possible for another process to be writing to the container 
while we're processing its metadata. The filesystem will ensure that we only 
ever see complete records, but the file size could change at any time."

Then, for specific information about the repro case, add a reference to the 
JIRA.


Line 504: 
Nit: remove this empty line?


-- 
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: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to