From 46cf5fbfdd3f948170a6cbfc19d3ade6e9e0285c Mon Sep 17 00:00:00 2001 From: Akanksha Mahajan Date: Mon, 22 Feb 2021 22:07:59 -0800 Subject: [PATCH] Extend VerifyFileChecksums API for blob files (#7979) Summary: Extend VerifyFileChecksums API to verify blob files in case of use_file_checksum. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7979 Test Plan: New unit test db_blob_corruption_test Reviewed By: ltamasi Differential Revision: D26534040 Pulled By: akankshamahajan15 fbshipit-source-id: 7dc5951a3df9d265ea1265e0122b43c966856ade --- CMakeLists.txt | 1 + Makefile | 3 ++ TARGETS | 7 +++ db/blob/db_blob_corruption_test.cc | 81 ++++++++++++++++++++++++++++++ db/corruption_test.cc | 2 +- db/db_impl/db_impl.cc | 42 ++++++++++++---- db/db_impl/db_impl.h | 7 +-- src.mk | 1 + 8 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 db/blob/db_blob_corruption_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index a22d70753..ef2b20deb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1061,6 +1061,7 @@ if(WITH_TESTS) db/blob/blob_file_garbage_test.cc db/blob/blob_file_reader_test.cc db/blob/db_blob_basic_test.cc + db/blob/db_blob_corruption_test.cc db/blob/db_blob_index_test.cc db/column_family_test.cc db/compact_files_test.cc diff --git a/Makefile b/Makefile index 23b2f84b8..8f42af139 100644 --- a/Makefile +++ b/Makefile @@ -592,6 +592,7 @@ ifdef ASSERT_STATUS_CHECKED db_log_iter_test \ db_bloom_filter_test \ db_blob_basic_test \ + db_blob_corruption_test \ db_blob_index_test \ db_block_cache_test \ db_compaction_test \ @@ -2059,6 +2060,8 @@ io_tracer_parser_test: $(OBJ_DIR)/tools/io_tracer_parser_test.o $(OBJ_DIR)/tools io_tracer_parser: $(OBJ_DIR)/tools/io_tracer_parser.o $(TOOLS_LIBRARY) $(LIBRARY) $(AM_LINK) +db_blob_corruption_test: $(OBJ_DIR)/db/blob/db_blob_corruption_test.o $(TEST_LIBRARY) $(LIBRARY) + $(AM_LINK) #------------------------------------------------- # make install related stuff PREFIX ?= /usr/local diff --git a/TARGETS b/TARGETS index 64e4547fb..30372b725 100644 --- a/TARGETS +++ b/TARGETS @@ -1149,6 +1149,13 @@ ROCKS_TESTS = [ [], [], ], + [ + "db_blob_corruption_test", + "db/blob/db_blob_corruption_test.cc", + "serial", + [], + [], + ], [ "db_blob_index_test", "db/blob/db_blob_index_test.cc", diff --git a/db/blob/db_blob_corruption_test.cc b/db/blob/db_blob_corruption_test.cc new file mode 100644 index 000000000..77f11b75a --- /dev/null +++ b/db/blob/db_blob_corruption_test.cc @@ -0,0 +1,81 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). + +#include "db/db_test_util.h" +#include "port/stack_trace.h" +#include "test_util/sync_point.h" + +namespace ROCKSDB_NAMESPACE { + +class DBBlobCorruptionTest : public DBTestBase { + protected: + DBBlobCorruptionTest() + : DBTestBase("/db_blob_corruption_test", /* env_do_fsync */ false) {} + + void Corrupt(FileType filetype, int offset, int bytes_to_corrupt) { + // Pick file to corrupt + std::vector filenames; + ASSERT_OK(env_->GetChildren(dbname_, &filenames)); + uint64_t number; + FileType type; + std::string fname; + uint64_t picked_number = kInvalidBlobFileNumber; + for (size_t i = 0; i < filenames.size(); i++) { + if (ParseFileName(filenames[i], &number, &type) && type == filetype && + number > picked_number) { // Pick latest file + fname = dbname_ + "/" + filenames[i]; + picked_number = number; + } + } + ASSERT_TRUE(!fname.empty()) << filetype; + ASSERT_OK(test::CorruptFile(env_, fname, offset, bytes_to_corrupt)); + } +}; + +#ifndef ROCKSDB_LITE +TEST_F(DBBlobCorruptionTest, VerifyWholeBlobFileChecksum) { + Options options = GetDefaultOptions(); + options.enable_blob_files = true; + options.min_blob_size = 0; + options.create_if_missing = true; + options.file_checksum_gen_factory = + ROCKSDB_NAMESPACE::GetFileChecksumGenCrc32cFactory(); + Reopen(options); + + ASSERT_OK(Put(Slice("key_1"), Slice("blob_value_1"))); + ASSERT_OK(Flush()); + ASSERT_OK(Put(Slice("key_2"), Slice("blob_value_2"))); + ASSERT_OK(Flush()); + ASSERT_OK(db_->VerifyFileChecksums(ReadOptions())); + Close(); + + Corrupt(kBlobFile, 0, 2); + + ASSERT_OK(TryReopen(options)); + + int count{0}; + SyncPoint::GetInstance()->SetCallBack( + "DBImpl::VerifyFullFileChecksum:mismatch", [&](void* arg) { + const Status* s = static_cast(arg); + ASSERT_NE(s, nullptr); + ++count; + ASSERT_NOK(*s); + }); + SyncPoint::GetInstance()->EnableProcessing(); + + ASSERT_TRUE(db_->VerifyFileChecksums(ReadOptions()).IsCorruption()); + ASSERT_EQ(1, count); + + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing(); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearAllCallBacks(); +} +#endif // !ROCKSDB_LITE +} // namespace ROCKSDB_NAMESPACE + +int main(int argc, char** argv) { + ROCKSDB_NAMESPACE::port::InstallStackTraceHandler(); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/db/corruption_test.cc b/db/corruption_test.cc index 4fb1f7403..96671b67a 100644 --- a/db/corruption_test.cc +++ b/db/corruption_test.cc @@ -882,7 +882,7 @@ TEST_F(CorruptionTest, VerifyWholeTableChecksum) { SyncPoint::GetInstance()->ClearAllCallBacks(); int count{0}; SyncPoint::GetInstance()->SetCallBack( - "DBImpl::VerifySstFileChecksum:mismatch", [&](void* arg) { + "DBImpl::VerifyFullFileChecksum:mismatch", [&](void* arg) { auto* s = reinterpret_cast(arg); ASSERT_NE(s, nullptr); ++count; diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 19b682b2c..1a5cdecd3 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -4855,17 +4855,37 @@ Status DBImpl::VerifyChecksumInternal(const ReadOptions& read_options, std::string fname = TableFileName(cfd->ioptions()->cf_paths, fd.GetNumber(), fd.GetPathId()); if (use_file_checksum) { - s = VerifySstFileChecksum(*fmeta, fname, read_options); + s = VerifyFullFileChecksum(fmeta->file_checksum, + fmeta->file_checksum_func_name, fname, + read_options); } else { s = ROCKSDB_NAMESPACE::VerifySstFileChecksum(opts, file_options_, read_options, fname); } } } + + if (s.ok() && use_file_checksum) { + const auto& blob_files = vstorage->GetBlobFiles(); + for (const auto& pair : blob_files) { + const uint64_t blob_file_number = pair.first; + const auto& meta = pair.second; + assert(meta); + const std::string blob_file_name = BlobFileName( + cfd->ioptions()->cf_paths.front().path, blob_file_number); + s = VerifyFullFileChecksum(meta->GetChecksumValue(), + meta->GetChecksumMethod(), blob_file_name, + read_options); + if (!s.ok()) { + break; + } + } + } if (!s.ok()) { break; } } + bool defer_purge = immutable_db_options().avoid_unnecessary_blocking_io; { @@ -4890,29 +4910,31 @@ Status DBImpl::VerifyChecksumInternal(const ReadOptions& read_options, return s; } -Status DBImpl::VerifySstFileChecksum(const FileMetaData& fmeta, - const std::string& fname, - const ReadOptions& read_options) { +Status DBImpl::VerifyFullFileChecksum(const std::string& file_checksum_expected, + const std::string& func_name_expected, + const std::string& fname, + const ReadOptions& read_options) { Status s; - if (fmeta.file_checksum == kUnknownFileChecksum) { + if (file_checksum_expected == kUnknownFileChecksum) { return s; } std::string file_checksum; std::string func_name; s = ROCKSDB_NAMESPACE::GenerateOneFileChecksum( fs_.get(), fname, immutable_db_options_.file_checksum_gen_factory.get(), - fmeta.file_checksum_func_name, &file_checksum, &func_name, + func_name_expected, &file_checksum, &func_name, read_options.readahead_size, immutable_db_options_.allow_mmap_reads, io_tracer_, immutable_db_options_.rate_limiter.get()); if (s.ok()) { - assert(fmeta.file_checksum_func_name == func_name); - if (file_checksum != fmeta.file_checksum) { + assert(func_name_expected == func_name); + if (file_checksum != file_checksum_expected) { std::ostringstream oss; oss << fname << " file checksum mismatch, "; - oss << "expecting " << Slice(fmeta.file_checksum).ToString(/*hex=*/true); + oss << "expecting " + << Slice(file_checksum_expected).ToString(/*hex=*/true); oss << ", but actual " << Slice(file_checksum).ToString(/*hex=*/true); s = Status::Corruption(oss.str()); - TEST_SYNC_POINT_CALLBACK("DBImpl::VerifySstFileChecksum:mismatch", &s); + TEST_SYNC_POINT_CALLBACK("DBImpl::VerifyFullFileChecksum:mismatch", &s); } } return s; diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 1973eceb2..9b732703c 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -447,9 +447,10 @@ class DBImpl : public DB { Status VerifyChecksumInternal(const ReadOptions& read_options, bool use_file_checksum); - Status VerifySstFileChecksum(const FileMetaData& fmeta, - const std::string& fpath, - const ReadOptions& read_options); + Status VerifyFullFileChecksum(const std::string& file_checksum_expected, + const std::string& func_name_expected, + const std::string& fpath, + const ReadOptions& read_options); using DB::StartTrace; virtual Status StartTrace( diff --git a/src.mk b/src.mk index cf4a694c9..d4c867c89 100644 --- a/src.mk +++ b/src.mk @@ -377,6 +377,7 @@ TEST_MAIN_SOURCES = \ db/blob/blob_file_garbage_test.cc \ db/blob/blob_file_reader_test.cc \ db/blob/db_blob_basic_test.cc \ + db/blob/db_blob_corruption_test.cc \ db/blob/db_blob_index_test.cc \ db/column_family_test.cc \ db/compact_files_test.cc \