From 0804b44fb69d81b18907bd2dd066ba90fe017e3c Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Wed, 28 Jul 2021 16:43:16 -0700 Subject: [PATCH] Some fixes and enhancements to `ldb repair` (#8544) Summary: * Basic handling of SST file with just range tombstones rather than failing assertion about smallest_seqno <= largest_seqno * Adds --verbose option so that there exists a way to see the INFO output from Repairer. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8544 Test Plan: unit test added, manual testing for --verbose Reviewed By: ajkr Differential Revision: D29954805 Pulled By: pdillinger fbshipit-source-id: 696af25805fc36cc178b04ba6045922a22625fd9 --- db/repair.cc | 26 ++++++++++++++++++++++++++ db/repair_test.cc | 30 ++++++++++++++++++++++++++++++ tools/ldb_cmd.cc | 10 ++++++++-- tools/ldb_cmd_impl.h | 6 ++++++ 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/db/repair.cc b/db/repair.cc index 8ca41d868..28f28d440 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -300,6 +300,8 @@ class Repairer { } for (size_t path_id = 0; path_id < to_search_paths.size(); path_id++) { + ROCKS_LOG_INFO(db_options_.info_log, "Searching path %s\n", + to_search_paths[path_id].c_str()); status = env_->GetChildren(to_search_paths[path_id], &filenames); if (!status.ok()) { return status; @@ -587,6 +589,30 @@ class Repairer { t->meta.fd.GetNumber(), counter, status.ToString().c_str()); } + if (status.ok()) { + // XXX/FIXME: This is just basic, naive handling of range tombstones, + // like call to UpdateBoundariesForRange in builder.cc where we assume + // an SST file is a full sorted run. This probably needs the extra logic + // from compaction_job.cc around call to UpdateBoundariesForRange (to + // handle range tombstones extendingg beyond range of other entries). + ReadOptions ropts; + std::unique_ptr r_iter; + status = table_cache_->GetRangeTombstoneIterator( + ropts, cfd->internal_comparator(), t->meta, &r_iter); + + if (r_iter) { + r_iter->SeekToFirst(); + + while (r_iter->Valid()) { + auto tombstone = r_iter->Tombstone(); + auto kv = tombstone.Serialize(); + t->meta.UpdateBoundariesForRange( + kv.first, tombstone.SerializeEndKey(), tombstone.seq_, + cfd->internal_comparator()); + r_iter->Next(); + } + } + } return status; } diff --git a/db/repair_test.cc b/db/repair_test.cc index 8e7551a98..2b9750052 100644 --- a/db/repair_test.cc +++ b/db/repair_test.cc @@ -3,6 +3,7 @@ // COPYING file in the root directory) and Apache 2.0 License // (found in the LICENSE.Apache file in the root directory). +#include "rocksdb/options.h" #ifndef ROCKSDB_LITE #include @@ -66,6 +67,35 @@ TEST_F(RepairTest, LostManifest) { ASSERT_EQ(Get("key2"), "val2"); } +TEST_F(RepairTest, LostManifestMoreDbFeatures) { + // Add a couple SST files, delete the manifest, and verify RepairDB() saves + // the day. + ASSERT_OK(Put("key", "val")); + ASSERT_OK(Put("key2", "val2")); + ASSERT_OK(Put("key3", "val3")); + ASSERT_OK(Put("key4", "val4")); + ASSERT_OK(Flush()); + // Test an SST file containing only a range tombstone + ASSERT_OK(db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "key2", + "key3z")); + ASSERT_OK(Flush()); + // Need to get path before Close() deletes db_, but delete it after Close() to + // ensure Close() didn't change the manifest. + std::string manifest_path = + DescriptorFileName(dbname_, dbfull()->TEST_Current_Manifest_FileNo()); + + Close(); + ASSERT_OK(env_->FileExists(manifest_path)); + ASSERT_OK(env_->DeleteFile(manifest_path)); + ASSERT_OK(RepairDB(dbname_, CurrentOptions())); + Reopen(CurrentOptions()); + + ASSERT_EQ(Get("key"), "val"); + ASSERT_EQ(Get("key2"), "NOT_FOUND"); + ASSERT_EQ(Get("key3"), "NOT_FOUND"); + ASSERT_EQ(Get("key4"), "val4"); +} + TEST_F(RepairTest, CorruptManifest) { // Manifest is in an invalid format. Expect a full recovery. ASSERT_OK(Put("key", "val")); diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 471dcbb5a..52de452f9 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -3079,20 +3079,26 @@ void CheckPointCommand::DoCommand() { // ---------------------------------------------------------------------------- +const std::string RepairCommand::ARG_VERBOSE = "verbose"; + RepairCommand::RepairCommand(const std::vector& /*params*/, const std::map& options, const std::vector& flags) - : LDBCommand(options, flags, false, BuildCmdLineOptions({})) {} + : LDBCommand(options, flags, false, BuildCmdLineOptions({ARG_VERBOSE})) { + verbose_ = IsFlagPresent(flags, ARG_VERBOSE); +} void RepairCommand::Help(std::string& ret) { ret.append(" "); ret.append(RepairCommand::Name()); + ret.append(" [--" + ARG_VERBOSE + "]"); ret.append("\n"); } void RepairCommand::OverrideBaseOptions() { LDBCommand::OverrideBaseOptions(); - options_.info_log.reset(new StderrLogger(InfoLogLevel::WARN_LEVEL)); + auto level = verbose_ ? InfoLogLevel::INFO_LEVEL : InfoLogLevel::WARN_LEVEL; + options_.info_log.reset(new StderrLogger(level)); } void RepairCommand::DoCommand() { diff --git a/tools/ldb_cmd_impl.h b/tools/ldb_cmd_impl.h index 9944dc11e..8e20f61f1 100644 --- a/tools/ldb_cmd_impl.h +++ b/tools/ldb_cmd_impl.h @@ -549,6 +549,12 @@ class RepairCommand : public LDBCommand { virtual void OverrideBaseOptions() override; static void Help(std::string& ret); + + protected: + bool verbose_; + + private: + static const std::string ARG_VERBOSE; }; class BackupableCommand : public LDBCommand {