Adar Dembo 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) { In MarkInvalidated() too. Line 173: std::atomic<uint8_t> flags_ {0}; This change was motivated by the new calls to invalidated() in ReopenFileIfNecessary(), right? Previously we could guarantee that the cache spinlock was always held during calls to deleted(), but that's not the case anymore. Line 317: CHECK(!base_.invalidated()); Should be before the lookup, no? Line 420: Nit: make empty line use consistent with the other ReopenFileIfNecessary(), since the functions are almost the same otherwise. Line 508: Status FileCache<FileType>::DeleteFile(const string& file_name) { To ward against concurrent Invalidate() and DeleteFile(), should we CHECK(!invalidated()) here too? It's not an issue in OpenExistingFile() because we'll notice the use of an invalidated descriptor during a subsequent file operation, but here there's no such thing. -- 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
