From 69a18b9bad5a1bc9b86ce2d1eac397efe9d9350e Mon Sep 17 00:00:00 2001 From: Jay Zhuang Date: Fri, 15 Jul 2022 11:50:30 -0700 Subject: [PATCH] VerifySstUniqueIds status is overrided for multi CFs (#10247) Summary: There's bug that basically we only report the last CF's VerifySstUniqueIds() result: https://github.com/facebook/rocksdb/pull/9990#discussion_r877268810 Pull Request resolved: https://github.com/facebook/rocksdb/pull/10247 Test Plan: CI Reviewed By: pdillinger Differential Revision: D37384265 Pulled By: jay-zhuang fbshipit-source-id: d462ad0eab39c9145c45a3db9c45539d5d76f7dd --- db/db_impl/db_impl_open.cc | 6 ++++++ db/db_test2.cc | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index c854c3870ba..69fd171833a 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -718,6 +718,12 @@ Status DBImpl::VerifySstUniqueIdInManifest() { status = version->VerifySstUniqueIds(); mutex_.Lock(); version->Unref(); + if (!status.ok()) { + ROCKS_LOG_WARN(immutable_db_options_.info_log, + "SST unique id mismatch in column family \"%s\": %s", + cfd->GetName().c_str(), status.ToString().c_str()); + return status; + } } } return status; diff --git a/db/db_test2.cc b/db/db_test2.cc index a6d6813b817..6e121f3621f 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -7436,6 +7436,45 @@ TEST_F(DBTest2, SstUniqueIdVerify) { ASSERT_TRUE(s.IsCorruption()); } +TEST_F(DBTest2, SstUniqueIdVerifyMultiCFs) { + const int kNumSst = 3; + const int kLevel0Trigger = 4; + auto options = CurrentOptions(); + options.level0_file_num_compaction_trigger = kLevel0Trigger; + + CreateAndReopenWithCF({"one", "two"}, options); + + // generate good SSTs + for (int cf_num : {0, 2}) { + for (int i = 0; i < kNumSst; i++) { + for (int j = 0; j < 100; j++) { + ASSERT_OK(Put(cf_num, Key(i * 10 + j), "value")); + } + ASSERT_OK(Flush(cf_num)); + } + } + + // generate SSTs with bad unique id + SyncPoint::GetInstance()->SetCallBack( + "PropertyBlockBuilder::AddTableProperty:Start", [&](void* props_vs) { + auto props = static_cast(props_vs); + // update table property session_id to a different one + props->db_session_id = DBImpl::GenerateDbSessionId(nullptr); + }); + SyncPoint::GetInstance()->EnableProcessing(); + for (int i = 0; i < kNumSst; i++) { + for (int j = 0; j < 100; j++) { + ASSERT_OK(Put(1, Key(i * 10 + j), "value")); + } + ASSERT_OK(Flush(1)); + } + + // Reopen with verification should report corruption + options.verify_sst_unique_id_in_manifest = true; + auto s = TryReopenWithColumnFamilies({"default", "one", "two"}, options); + ASSERT_TRUE(s.IsCorruption()); +} + #ifndef ROCKSDB_LITE TEST_F(DBTest2, GetLatestSeqAndTsForKey) { Destroy(last_options_);