Todd Lipcon has posted comments on this change. Change subject: log_block_manager: fix corruption after re-opening compacted metadata ......................................................................
Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7113/3/src/kudu/util/file_cache.cc File src/kudu/util/file_cache.cc: Line 525: void FileCache<FileType>::Invalidate(const string& file_name) { > What about a concurrent DeleteFile() call? Should DeleteFile() also return Plausibly? But really I think all of this should be "YMMV, don't do stuff concurrently to a file". All of our use cases for this are externally synchronized already (i.e our one use case is done at startup). How do you feel about just making an admonishment in the function doc that it is not safe to use concurrently with other operations on the same path, and the "invalidation" thing is just a safeguard so that if someone tries to do it they'll get an error? I could even change it to be a CHECK failure to use an invalidated fd if that feels better? Basically I don't want to grow an extra concurrency feature (and associated tests, docs, etc) that we don't actually need, especially considering this is also the long pole in a release right now. -- 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: 3 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
