Todd Lipcon has posted comments on this change. Change subject: log_block_manager: fix corruption after re-opening compacted metadata ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/7113/1/src/kudu/fs/log_block_manager-test.cc File src/kudu/fs/log_block_manager-test.cc: Line 1208: TEST_F(LogBlockManagerTest, TestDeleteFromContainerAfterMetadataCompaction) { > Maybe add the word "Fuzz" in there, since it's sort of a fuzz test (i.e. th hrm, I don't think of this really as a fuzz test because it's not generating some complicated sequence or bit pattern. You're right it's not 100% deterministic, but neither are most of our tests and we don't label them all fuzz. http://gerrit.cloudera.org:8080/#/c/7113/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 2330: file_cache_.Evict(metadata_file_name); > Curious as to whether you thought about implementing the rename through the I did think about it, but seemed like it would be a bit more complicated to support error handling cases, etc. I changed it around a bit to be "safer", though. Now, the method is Invalidate(), and any existing open descriptors are marked such that if you try to continue using them post-invalidation you get an error rather than transparently re-opening the new file. http://gerrit.cloudera.org:8080/#/c/7113/1/src/kudu/util/file_cache-test.cc File src/kudu/util/file_cache-test.cc: Line 227: // Create a test file, then delete it. It will be deleted immediately. > Comment doesn't really make sense; we're renaming the file, not deleting it Copy pasta. done. http://gerrit.cloudera.org:8080/#/c/7113/1/src/kudu/util/file_cache.h File src/kudu/util/file_cache.h: PS1, Line 147: It's recommended that the above pattern : // only be used in cases when it can be guaranteed that no other objects : // retain an open descriptor to the same path. > Can we enforce that there are no open readers of this file at the time of e I changed it around to mark the existing descriptor as 'invalidated' and then ensure that any attempt to use an invalidated descriptor gets an illegal-state error. -- 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: 1 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
