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 committed by Facebook GitHub Bot
parent 9bfd46d0d8
commit b7bab48099
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

@ -2,6 +2,7 @@
## Unreleased ## Unreleased
### Behavior Changes ### Behavior Changes
* 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.
## 6.11 (6/12/2020) ## 6.11 (6/12/2020)
### Bug Fixes ### Bug Fixes

10
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,8 +559,9 @@ 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() && force_create_version))) { ((!missing_files.empty() && !prev_has_missing_files) ||
(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());
auto* builder = builder_iter->second->version_builder(); auto* builder = builder_iter->second->version_builder();