Handle blob files when options.best_efforts_recovery is true #8180
Conversation
| @@ -267,6 +267,51 @@ TEST_F(DBBlobBasicTest, GenerateIOTracing) { | |||
| } | |||
| #endif // !ROCKSDB_LITE | |||
|
|
|||
| TEST_F(DBBlobBasicTest, BestEffortsRecovery_MissingNewestBlobFile) { | |||
There was a problem hiding this comment.
Could you add some comments here to explain what's happening in this test case? IIUC we flush four SST file-blob file pairs, then delete the blob file from the fourth one, resulting in the Version created by the third flush being the latest recoverable one. Right?
P.S. I've just realized we write just enough SSTs to trigger a compaction; I'm wondering if that's necessary.
There was a problem hiding this comment.
Originally, I added the compaction because I would like to demonstrate that no new blob files are generated during this test even with compaction, but I think you are right too. It's not necessary.
| @@ -437,6 +439,8 @@ ColumnFamilyData* VersionEditHandler::CreateCfAndInit( | |||
| if (track_missing_files_) { | |||
| cf_to_missing_files_.emplace(edit.column_family_, | |||
| std::unordered_set<uint64_t>()); | |||
| cf_to_missing_blob_files_high_.emplace(edit.column_family_, | |||
There was a problem hiding this comment.
Should we update VersionEditHandler::HasMissingFiles?
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
db/version_edit_handler.cc
Outdated
| // MANIFEST. We check whether we have any missing table and blob files. | ||
| const bool prev_has_missing_files = | ||
| !missing_files.empty() || | ||
| (builder && |
There was a problem hiding this comment.
How about replacing the non-null check for builder with prev_missing_blob_file_high != kInvalidBlobFileNumber? The latter implies the former and might be easier to understand... I.e.:
const bool prev_has_missing_files = !missing_files.empty() || (prev_missing_blob_file_high != kInvalidBlobFileNumber && prev_missing_blob_file_high >= builder->GetMinOldestBlobFileNumber());
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@riversand963 merged this pull request in b0e2019. |
If
options.best_efforts_recovery == true, RocksDB currently tolerates missing table files and recovers to the latest version without missing table files (not considering WAL). It is necessary to handle blob files as well to make the feature more complete.Test plan
make check