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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
