diff --git a/HISTORY.md b/HISTORY.md index aa7d328a7e3..aeaa1f8138a 100644 --- a/HISTORY.md +++ b/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. diff --git a/db/version_set.cc b/db/version_set.cc index 5f35434bfeb..e487b6a014e 100644 --- a/db/version_set.cc +++ b/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(&storage_info_.force_consistency_checks_)); UpdateAccumulatedStats(update_stats); storage_info_.UpdateNumNonEmptyLevels(); storage_info_.CalculateBaseBytes(*cfd_->ioptions(), mutable_cf_options); diff --git a/include/rocksdb/utilities/ldb_cmd.h b/include/rocksdb/utilities/ldb_cmd.h index 0942db22151..874d36e185b 100644 --- a/include/rocksdb/utilities/ldb_cmd.h +++ b/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_; /** diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 661fc0fc671..c8033709c22 100644 --- a/tools/ldb_cmd.cc +++ b/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& 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 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& /*params*/, const std::map& options, const std::vector& 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(); } // ---------------------------------------------------------------------------- diff --git a/tools/ldb_cmd_test.cc b/tools/ldb_cmd_test.cc index 8bc9c438aba..638ef9568e7 100644 --- a/tools/ldb_cmd_test.cc +++ b/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(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(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(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(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 diff --git a/tools/ldb_tool.cc b/tools/ldb_tool.cc index 8174b7e0c72..80e71b35fba 100644 --- a/tools/ldb_tool.cc +++ b/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 + "=\n");