Improve ldb consistency checks (#6802)

Summary:
When using ldb, users cannot turn on force consistency check in most commands, while they cannot use checksonsistnecy with --try_load_options. The change fixes both by:
1. checkconsistency now calls OpenDB() so that it gets all the options loading and sanitized options logic
2. use options.check_consistency_checks = true by default, and add a --disable_consistency_checks to turn it off.
Pull Request resolved: #6802

Test Plan: Add a new unit test. Some manual tests with corrupted DBs.

Reviewed By: pdillinger

Differential Revision: D21388051

fbshipit-source-id: 8d122732d391b426e3982a1c3232a8e3763ffad0

HISTORY.md

@@ -17,12 +17,16 @@
 * Fix corruption caused by enabling delete triggered compaction (NewCompactOnDeletionCollectorFactory) in universal compaction mode, along with parallel compactions. The bug can result in two parallel compactions picking the same input files, resulting in the DB resurrecting older and deleted versions of some keys.
 * Fix a use-after-free bug in best-efforts recovery. column_family_memtables_ needs to point to valid ColumnFamilySet.
 * Let best-efforts recovery ignore corrupted files during table loading.
+* Fix a bug when making options.bottommost_compression, options.compression_opts and options.bottommost_compression_opts dynamically changeable: the modified values are not written to option files or returned back to users when being queried.
+* Fix a bug where index key comparisons were unaccounted in `PerfContext::user_key_comparison_count` for lookups in files written with `format_version >= 3`.
+* Fix many bloom.filter statistics not being updated in batch MultiGet.
 
 
 ### 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 NewFileChecksumGenCrc32cFactory to the file checksum public API, such that the builtin Crc32c based file checksum generator factory can be used by applications.
 * Add IsDirectory to Env and FS to indicate if a path is a directory.
+* ldb now uses options.force_consistency_checks = true by default and "--disable_consistency_checks" is added to disable it.
 
 ### New Features
 * Added support for pipelined & parallel compression optimization for `BlockBasedTableBuilder`. This optimization makes block building, block compression and block appending a pipeline, and uses multiple threads to accelerate block compression. Users can set `CompressionOptions::parallel_threads` greater than 1 to enable compression parallelism. This feature is experimental for now.
@@ -31,11 +35,6 @@
 * Added functionality in sst_dump tool to check the compressed file size for different compression levels and print the time spent on compressing files with each compression type. Added arguments `--compression_level_from` and `--compression_level_to` to report size of all compression levels and one compression_type must be specified with it so that it will report compressed sizes of one compression type with different levels.
 * Added statistics for redundant insertions into block cache: rocksdb.block.cache.*add.redundant. (There is currently no coordination to ensure that only one thread loads a table block when many threads are trying to access that same table block.)
 
-### Bug Fixes
-* Fix a bug when making options.bottommost_compression, options.compression_opts and options.bottommost_compression_opts dynamically changeable: the modified values are not written to option files or returned back to users when being queried.
-* Fix a bug where index key comparisons were unaccounted in `PerfContext::user_key_comparison_count` for lookups in files written with `format_version >= 3`.
-* Fix many bloom.filter statistics not being updated in batch MultiGet.
-
 ### Performance Improvements
 * Improve performance of batch MultiGet with partitioned filters, by sharing block cache lookups to applicable filter blocks.
 * Reduced memory copies when fetching and uncompressing compressed blocks from sst files.

db/version_set.cc

@@ -2087,6 +2087,9 @@ void VersionStorageInfo::GenerateLevelFilesBrief() {
 void Version::PrepareApply(
  const MutableCFOptions& mutable_cf_options,
  bool update_stats) {
+ TEST_SYNC_POINT_CALLBACK(
+ "Version::PrepareApply:forced_check",
+ reinterpret_cast<void*>(&storage_info_.force_consistency_checks_));
  UpdateAccumulatedStats(update_stats);
  storage_info_.UpdateNumNonEmptyLevels();
  storage_info_.CalculateBaseBytes(*cfd_->ioptions(), mutable_cf_options);

include/rocksdb/utilities/ldb_cmd.h

@@ -59,6 +59,7 @@ class LDBCommand {
  static const std::string ARG_FILE_SIZE;
  static const std::string ARG_CREATE_IF_MISSING;
  static const std::string ARG_NO_VALUE;
+ static const std::string ARG_DISABLE_CONSISTENCY_CHECKS;
 
  struct ParsedParams {
  std::string cmd;
@@ -163,6 +164,9 @@ class LDBCommand {
  // If true, try to construct options from DB's option files.
  bool try_load_options_;
 
+ // The value passed to options.force_consistency_checks.
+ bool force_consistency_checks_;
+
  bool create_if_missing_;
 
  /**

tools/ldb_cmd.cc

@@ -64,6 +64,8 @@ const std::string LDBCommand::ARG_TTL_START = "start_time";
 const std::string LDBCommand::ARG_TTL_END = "end_time";
 const std::string LDBCommand::ARG_TIMESTAMP = "timestamp";
 const std::string LDBCommand::ARG_TRY_LOAD_OPTIONS = "try_load_options";
+const std::string LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS =
+ "disable_consistency_checks";
 const std::string LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS =
  "ignore_unknown_options";
 const std::string LDBCommand::ARG_FROM = "from";
@@ -362,6 +364,8 @@ LDBCommand::LDBCommand(const std::map<std::string, std::string>& options,
  is_db_ttl_ = IsFlagPresent(flags, ARG_TTL);
  timestamp_ = IsFlagPresent(flags, ARG_TIMESTAMP);
  try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS);
+ force_consistency_checks_ =
+ !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS);
  config_options_.ignore_unknown_options =
  IsFlagPresent(flags, ARG_IGNORE_UNKNOWN_OPTIONS);
 }
@@ -527,6 +531,7 @@ std::vector<std::string> LDBCommand::BuildCmdLineOptions(
  ARG_FILE_SIZE,
  ARG_FIX_PREFIX_LEN,
  ARG_TRY_LOAD_OPTIONS,
+ ARG_DISABLE_CONSISTENCY_CHECKS,
  ARG_IGNORE_UNKNOWN_OPTIONS,
  ARG_CF_NAME};
  ret.insert(ret.end(), options.begin(), options.end());
@@ -622,6 +627,7 @@ Options LDBCommand::PrepareOptionsForOpenDB() {
  }
  }
 
+ cf_opts->force_consistency_checks = force_consistency_checks_;
  if (use_table_options) {
  cf_opts->table_factory.reset(NewBlockBasedTableFactory(table_options));
  }
@@ -2839,7 +2845,7 @@ CheckConsistencyCommand::CheckConsistencyCommand(
  const std::vector<std::string>& /*params*/,
  const std::map<std::string, std::string>& options,
  const std::vector<std::string>& flags)
- : LDBCommand(options, flags, false, BuildCmdLineOptions({})) {}
+ : LDBCommand(options, flags, true, BuildCmdLineOptions({})) {}
 
 void CheckConsistencyCommand::Help(std::string& ret) {
  ret.append(" ");
@@ -2848,19 +2854,13 @@ void CheckConsistencyCommand::Help(std::string& ret) {
 }
 
 void CheckConsistencyCommand::DoCommand() {
- Options opt = PrepareOptionsForOpenDB();
- opt.paranoid_checks = true;
- if (!exec_state_.IsNotStarted()) {
- return;
- }
- DB* db;
- Status st = DB::OpenForReadOnly(opt, db_path_, &db, false);
- delete db;
- if (st.ok()) {
+ options_.paranoid_checks = true;
+ options_.num_levels = 64;
+ OpenDB();
+ if (exec_state_.IsSucceed() || exec_state_.IsNotStarted()) {
  fprintf(stdout, "OK\n");
- } else {
- exec_state_ = LDBCommandExecuteResult::Failed(st.ToString());
  }
+ CloseDB();
 }
 
 // ----------------------------------------------------------------------------

tools/ldb_cmd_test.cc

@@ -551,6 +551,98 @@ TEST_F(LdbCmdTest, ListFileTombstone) {
  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
  }
 }
+
+TEST_F(LdbCmdTest, DisableConsistencyChecks) {
+ Env* base_env = TryLoadCustomOrDefaultEnv();
+ std::unique_ptr<Env> env(NewMemEnv(base_env));
+ Options opts;
+ opts.env = env.get();
+ opts.create_if_missing = true;
+
+ std::string dbname = test::TmpDir();
+
+ {
+ DB* db = nullptr;
+ ASSERT_OK(DB::Open(opts, dbname, &db));
+
+ WriteOptions wopts;
+ FlushOptions fopts;
+ fopts.wait = true;
+
+ ASSERT_OK(db->Put(wopts, "foo1", "1"));
+ ASSERT_OK(db->Put(wopts, "bar1", "2"));
+ ASSERT_OK(db->Flush(fopts));
+
+ ASSERT_OK(db->Put(wopts, "foo2", "3"));
+ ASSERT_OK(db->Put(wopts, "bar2", "4"));
+ ASSERT_OK(db->Flush(fopts));
+
+ delete db;
+ }
+
+ {
+ char arg1[] = "./ldb";
+ char arg2[1024];
+ snprintf(arg2, sizeof(arg2), "--db=%s", dbname.c_str());
+ char arg3[] = "checkconsistency";
+ char* argv[] = {arg1, arg2, arg3};
+
+ SyncPoint::GetInstance()->SetCallBack(
+ "Version::PrepareApply:forced_check", [&](void* arg) {
+ bool* forced = reinterpret_cast<bool*>(arg);
+ ASSERT_TRUE(*forced);
+ });
+ SyncPoint::GetInstance()->EnableProcessing();
+
+ ASSERT_EQ(
+ 0, LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
+
+ SyncPoint::GetInstance()->ClearAllCallBacks();
+ SyncPoint::GetInstance()->DisableProcessing();
+ }
+ {
+ char arg1[] = "./ldb";
+ char arg2[1024];
+ snprintf(arg2, sizeof(arg2), "--db=%s", dbname.c_str());
+ char arg3[] = "scan";
+ char* argv[] = {arg1, arg2, arg3};
+
+ SyncPoint::GetInstance()->SetCallBack(
+ "Version::PrepareApply:forced_check", [&](void* arg) {
+ bool* forced = reinterpret_cast<bool*>(arg);
+ ASSERT_TRUE(*forced);
+ });
+ SyncPoint::GetInstance()->EnableProcessing();
+
+ ASSERT_EQ(
+ 0, LDBCommandRunner::RunCommand(3, argv, opts, LDBOptions(), nullptr));
+
+ SyncPoint::GetInstance()->ClearAllCallBacks();
+ SyncPoint::GetInstance()->DisableProcessing();
+ }
+ {
+ char arg1[] = "./ldb";
+ char arg2[1024];
+ snprintf(arg2, sizeof(arg2), "--db=%s", dbname.c_str());
+ char arg3[] = "scan";
+ char arg4[] = "--disable_consistency_checks";
+ char* argv[] = {arg1, arg2, arg3, arg4};
+
+ SyncPoint::GetInstance()->SetCallBack(
+ "ColumnFamilyData::ColumnFamilyData", [&](void* arg) {
+ ColumnFamilyOptions* cfo =
+ reinterpret_cast<ColumnFamilyOptions*>(arg);
+ ASSERT_FALSE(cfo->force_consistency_checks);
+ });
+ SyncPoint::GetInstance()->EnableProcessing();
+
+ ASSERT_EQ(
+ 0, LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr));
+
+ SyncPoint::GetInstance()->ClearAllCallBacks();
+ SyncPoint::GetInstance()->DisableProcessing();
+ }
+}
 } // namespace ROCKSDB_NAMESPACE
 
 #ifdef ROCKSDB_UNITTESTS_WITH_CUSTOM_OBJECTS_FROM_STATIC_LIBS

tools/ldb_tool.cc

@@ -46,6 +46,8 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options,
  " : DB supports ttl and value is internally timestamp-suffixed\n");
  ret.append(" --" + LDBCommand::ARG_TRY_LOAD_OPTIONS +
  " : Try to load option file from DB.\n");
+ ret.append(" --" + LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS +
+ " : Set options.force_consistency_checks = false.\n");
  ret.append(" --" + LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS +
  " : Ignore unknown options when loading option file.\n");
  ret.append(" --" + LDBCommand::ARG_BLOOM_BITS + "=<int,e.g.:14>\n");