Destroy any ColumnFamilyHandles in BlobDB::Open upon error (#6763)

Summary:
If an error happens during BlobDBImpl::Open after the base DB has been
opened, we need to destroy the `ColumnFamilyHandle`s returned by `DB::Open`
to prevent an assertion in `ColumnFamilySet`'s destructor from being hit.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6763

Test Plan: Ran `make check` and tested using the BlobDB mode of `db_bench`.

Reviewed By: riversand963

Differential Revision: D21262643

Pulled By: ltamasi

fbshipit-source-id: 60ebc7ab19be66cf37fbe5f6d8957d58470f3d3b
This commit is contained in:
Levi Tamasi 2020-04-27 16:42:16 -07:00 committed by Facebook GitHub Bot
parent cc8d16efd6
commit bea91d5d61
2 changed files with 11 additions and 0 deletions

View File

@ -4,6 +4,7 @@
* Fix wrong result being read from ingested file. May happen when a key in the file happen to be prefix of another key also in the file. The issue can further cause more data corruption. The issue exists with rocksdb >= 5.0.0 since DB::IngestExternalFile() was introduced. * Fix wrong result being read from ingested file. May happen when a key in the file happen to be prefix of another key also in the file. The issue can further cause more data corruption. The issue exists with rocksdb >= 5.0.0 since DB::IngestExternalFile() was introduced.
* Finish implementation of BlockBasedTableOptions::IndexType::kBinarySearchWithFirstKey. It's now ready for use. Significantly reduces read amplification in some setups, especially for iterator seeks. * Finish implementation of BlockBasedTableOptions::IndexType::kBinarySearchWithFirstKey. It's now ready for use. Significantly reduces read amplification in some setups, especially for iterator seeks.
* Fix a bug by updating CURRENT file so that it points to the correct MANIFEST file after best-efforts recovery. * Fix a bug by updating CURRENT file so that it points to the correct MANIFEST file after best-efforts recovery.
* Fixed a bug where ColumnFamilyHandle objects were not cleaned up in case an error happened during BlobDB's open after the base DB had been opened.
### Public API Change ### Public API Change
* Add a ConfigOptions argument to the APIs dealing with converting options to and from strings and files. The ConfigOptions is meant to replace some of the options (such as input_strings_escaped and ignore_unknown_options) and allow for more parameters to be passed in the future without changing the function signature. * Add a ConfigOptions argument to the APIs dealing with converting options to and from strings and files. The ConfigOptions is meant to replace some of the options (such as input_strings_escaped and ignore_unknown_options) and allow for more parameters to be passed in the future without changing the function signature.

View File

@ -38,6 +38,8 @@ Status BlobDB::Open(const DBOptions& db_options,
const std::vector<ColumnFamilyDescriptor>& column_families, const std::vector<ColumnFamilyDescriptor>& column_families,
std::vector<ColumnFamilyHandle*>* handles, std::vector<ColumnFamilyHandle*>* handles,
BlobDB** blob_db) { BlobDB** blob_db) {
assert(handles);
if (column_families.size() != 1 || if (column_families.size() != 1 ||
column_families[0].name != kDefaultColumnFamilyName) { column_families[0].name != kDefaultColumnFamilyName) {
return Status::NotSupported( return Status::NotSupported(
@ -50,6 +52,14 @@ Status BlobDB::Open(const DBOptions& db_options,
if (s.ok()) { if (s.ok()) {
*blob_db = static_cast<BlobDB*>(blob_db_impl); *blob_db = static_cast<BlobDB*>(blob_db_impl);
} else { } else {
if (!handles->empty()) {
for (ColumnFamilyHandle* cfh : *handles) {
blob_db_impl->DestroyColumnFamilyHandle(cfh);
}
handles->clear();
}
delete blob_db_impl; delete blob_db_impl;
*blob_db = nullptr; *blob_db = nullptr;
} }