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

Deprecating skip_log_error_on_recovery

Browse Source 
Summary:
Since https://reviews.facebook.net/D16119, we ignore partial tailing writes. Because of that, we no longer need skip_log_error_on_recovery.

The documentation says "Skip log corruption error on recovery (If client is ok with losing most recent changes)", while the option actually ignores any corruption of the WAL (not only just the most recent changes). This is very dangerous and can lead to DB inconsistencies. This was originally set up to ignore partial tailing writes, which we now do automatically (after D16119). I have digged up old task t2416297 which confirms my findings.

Test Plan: There was actually no tests that verified correct behavior of skip_log_error_on_recovery.

Reviewers: yhchiang, rven, dhruba, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D30603
This commit is contained in:
Igor Canadi 2015-01-05 13:35:56 -08:00
parent fa0b126c0c
commit 62ad0a9b19
4 changed files with 6 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
HISTORY.md
View File

@ -9,6 +9,9 @@
numbered levels will be placed later in the db_paths vector.
* Potentially big performance improvements if you're using RocksDB with lots of column families (100-1000)
### Public API changes
* Deprecated skip_log_error_on_recovery option
### 3.9.0 (12/8/2014)
### New Features

8
db/db_impl.cc
View File

@ -842,8 +842,7 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& log_numbers,
Env* env;
Logger* info_log;
const char* fname;
Status* status; // nullptr if db_options_.paranoid_checks==false or
// db_options_.skip_log_error_on_recovery==true
Status* status; // nullptr if db_options_.paranoid_checks==false
virtual void Corruption(size_t bytes, const Status& s) {
Log(InfoLogLevel::WARN_LEVEL,
info_log, "%s%s: dropping %d bytes; %s",
@ -888,10 +887,7 @@ Status DBImpl::RecoverLogFiles(const std::vector<uint64_t>& log_numbers,
reporter.env = env_;
reporter.info_log = db_options_.info_log.get();
reporter.fname = fname.c_str();
reporter.status =
(db_options_.paranoid_checks && !db_options_.skip_log_error_on_recovery
? &status
: nullptr);
reporter.status = (db_options_.paranoid_checks) ? &status : nullptr;
// We intentially make log::Reader do checksumming even if
// paranoid_checks==false so that corruptions cause entire commits
// to be skipped instead of propagating bad information (like overly

4
include/rocksdb/options.h
View File

@ -850,9 +850,7 @@ struct DBOptions {
// Disable child process inherit open files. Default: true
bool is_fd_close_on_exec;
// Skip log corruption error on recovery (If client is ok with
// losing most recent changes)
// Default: false
// DEPRECATED -- this options is no longer used
bool skip_log_error_on_recovery;
// if not zero, dump rocksdb.stats to LOG every stats_dump_period_sec

2
util/options.cc
View File

@ -333,8 +333,6 @@ void DBOptions::Dump(Logger* log) const {
allow_mmap_writes);
Log(log, " Options.is_fd_close_on_exec: %d",
is_fd_close_on_exec);
Log(log, " Options.skip_log_error_on_recovery: %d",
skip_log_error_on_recovery);
Log(log, " Options.stats_dump_period_sec: %u",
stats_dump_period_sec);
Log(log, " Options.advise_random_on_open: %d",