Todd Lipcon has posted comments on this change. Change subject: log_block_manager: fix corruption after re-opening compacted metadata ......................................................................
Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/7113/5/src/kudu/util/file_cache.cc File src/kudu/util/file_cache.cc: Line 141: while (true){ > Nit: while (true) { Done Line 173: std::atomic<uint8_t> flags_ {0}; > This change was motivated by the new calls to invalidated() in ReopenFileI right, TSAN build failed in the previous precommit Line 317: CHECK(!base_.invalidated()); > Should be before the lookup, no? I think the actual order in which the things happen, it's better to do the check _after_ we grabbed the fd. otherwise we could see "not invalidated", then grab the FD and get the FD that has been re-added for the new file. seems I messed this up below though. Line 420: > Nit: make empty line use consistent with the other ReopenFileIfNecessary(), Done Line 508: Status FileCache<FileType>::DeleteFile(const string& file_name) { > To ward against concurrent Invalidate() and DeleteFile(), should we CHECK(! added the CHECK in FindDescriptorUnlocked which is called below -- To view, visit http://gerrit.cloudera.org:8080/7113 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I14b2c64685e24d27591258911db4aeb9e8020a4d Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
