andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix a bug of overwriting return code (#6989)

Browse Source 
Summary:
In best-efforts recovery, an error that is not Corruption or IOError::kNotFound or IOError::kPathNotFound will be overwritten silently. Fix this by checking all non-ok cases and return early.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6989

Test Plan: make check

Reviewed By: ajkr

Differential Revision: D22071418

Pulled By: riversand963

fbshipit-source-id: 5a4ea5dfb1a41f41c7a3fdaf62b163007b42f04b
This commit is contained in:
Yanqin Jin 2020-06-16 12:56:43 -07:00
parent 97a69f4372
commit a818699f2f
2 changed files with 7 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
HISTORY.md
View File

@ -13,6 +13,7 @@
* Fix sst_dump to return non-zero exit code if the specified file is not a recognized SST file or fails requested checks. * Fix sst_dump to return non-zero exit code if the specified file is not a recognized SST file or fails requested checks.
* Fix incorrect results from batched MultiGet for duplicate keys, when the duplicate key matches the largest key of an SST file and the value type for the key in the file is a merge value. * Fix incorrect results from batched MultiGet for duplicate keys, when the duplicate key matches the largest key of an SST file and the value type for the key in the file is a merge value.
* Best-efforts recovery ignores CURRENT file completely. If CURRENT file is missing during recovery, best-efforts recovery still proceeds with MANIFEST file(s). * Best-efforts recovery ignores CURRENT file completely. If CURRENT file is missing during recovery, best-efforts recovery still proceeds with MANIFEST file(s).
* In best-efforts recovery, an error that is not Corruption or IOError::kNotFound or IOError::kPathNotFound will be overwritten silently. Fix this by checking all non-ok cases and return early.
### Public API Change ### Public API Change
* Flush(..., column_family) may return Status::ColumnFamilyDropped() instead of Status::InvalidArgument() if column_family is dropped while processing the flush request. * Flush(..., column_family) may return Status::ColumnFamilyDropped() instead of Status::InvalidArgument() if column_family is dropped while processing the flush request.

8
db/version_edit_handler.cc
View File

@ -404,13 +404,12 @@ Status VersionEditHandler::LoadTables(ColumnFamilyData* cfd,
bool is_initial_load) { bool is_initial_load) {
assert(cfd != nullptr); assert(cfd != nullptr);
assert(!cfd->IsDropped()); assert(!cfd->IsDropped());
Status s;
auto builder_iter = builders_.find(cfd->GetID()); auto builder_iter = builders_.find(cfd->GetID());
assert(builder_iter != builders_.end()); assert(builder_iter != builders_.end());
assert(builder_iter->second != nullptr); assert(builder_iter->second != nullptr);
VersionBuilder* builder = builder_iter->second->version_builder(); VersionBuilder* builder = builder_iter->second->version_builder();
assert(builder); assert(builder);
s = builder->LoadTableHandlers( Status s = builder->LoadTableHandlers(
cfd->internal_stats(), cfd->internal_stats(),
version_set_->db_options_->max_file_opening_threads, version_set_->db_options_->max_file_opening_threads,
prefetch_index_and_filter_in_cache, is_initial_load, prefetch_index_and_filter_in_cache, is_initial_load,
@ -551,6 +550,8 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersion(
if (s.IsPathNotFound() || s.IsNotFound() || s.IsCorruption()) { if (s.IsPathNotFound() || s.IsNotFound() || s.IsCorruption()) {
missing_files.insert(file_num); missing_files.insert(file_num);
s = Status::OK(); s = Status::OK();
} else if (!s.ok()) {
break;
} }
} }
bool missing_info = !version_edit_params_.has_log_number_ || bool missing_info = !version_edit_params_.has_log_number_ ||
@ -558,7 +559,8 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersion(
!version_edit_params_.has_last_sequence_; !version_edit_params_.has_last_sequence_;
// Create version before apply edit // Create version before apply edit
if (!missing_info && ((!missing_files.empty() && !prev_has_missing_files) || if (s.ok() && !missing_info &&
((!missing_files.empty() && !prev_has_missing_files) ||
(missing_files.empty() && force_create_version))) { (missing_files.empty() && force_create_version))) {
auto builder_iter = builders_.find(cfd->GetID()); auto builder_iter = builders_.find(cfd->GetID());
assert(builder_iter != builders_.end()); assert(builder_iter != builders_.end());