From 3020df9df59458fad384e80ffa069350e0b6cf6b Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Sun, 7 Jun 2020 21:54:54 -0700 Subject: [PATCH] Remove unnecessary inclusion of version_edit.h in env (#6952) Summary: In db_options.c, we should avoid including header files in the `db` directory to avoid introducing unnecessary dependency. The reason why `version_edit.h` has been included in `db_options.cc` is because we need two constants, `kUnknownChecksum` and `kUnknownChecksumFuncName`. We can put these two constants as `constexpr` in the public header `file_checksum.h`. Test plan (devserver): make check Pull Request resolved: https://github.com/facebook/rocksdb/pull/6952 Reviewed By: zhichao-cao Differential Revision: D21925341 Pulled By: riversand963 fbshipit-source-id: 2902f3b74c97f0cf16c58ad24c095c787c3a40e2 --- db/version_edit.cc | 4 ---- db/version_edit.h | 3 --- file/writable_file_writer.cc | 2 +- include/rocksdb/file_checksum.h | 5 +++++ options/db_options.cc | 6 ++---- table/block_based/block_based_table_builder.cc | 2 +- table/cuckoo/cuckoo_table_builder.cc | 2 +- table/mock_table.h | 2 +- table/plain/plain_table_builder.cc | 2 +- table/table_test.cc | 10 ++++------ 10 files changed, 16 insertions(+), 22 deletions(-) diff --git a/db/version_edit.cc b/db/version_edit.cc index c463f06c8..528c03c95 100644 --- a/db/version_edit.cc +++ b/db/version_edit.cc @@ -18,10 +18,6 @@ #include "util/string_util.h" namespace ROCKSDB_NAMESPACE { -// The unknown file checksum. -const std::string kUnknownFileChecksum(""); -// The unknown sst file checksum function name. -const std::string kUnknownFileChecksumFuncName("Unknown"); namespace { diff --git a/db/version_edit.h b/db/version_edit.h index 59e0e7a2a..0ca022854 100644 --- a/db/version_edit.h +++ b/db/version_edit.h @@ -29,9 +29,6 @@ constexpr uint64_t kFileNumberMask = 0x3FFFFFFFFFFFFFFF; constexpr uint64_t kUnknownOldestAncesterTime = 0; constexpr uint64_t kUnknownFileCreationTime = 0; -extern const std::string kUnknownFileChecksum; -extern const std::string kUnknownFileChecksumFuncName; - extern uint64_t PackFileNumberAndPathId(uint64_t number, uint64_t path_id); // A copyable structure contains information needed to read data from an SST diff --git a/file/writable_file_writer.cc b/file/writable_file_writer.cc index 66fcf9540..8e3bb131c 100644 --- a/file/writable_file_writer.cc +++ b/file/writable_file_writer.cc @@ -236,7 +236,7 @@ const char* WritableFileWriter::GetFileChecksumFuncName() const { if (checksum_generator_ != nullptr) { return checksum_generator_->Name(); } else { - return kUnknownFileChecksumFuncName.c_str(); + return kUnknownFileChecksumFuncName; } } diff --git a/include/rocksdb/file_checksum.h b/include/rocksdb/file_checksum.h index 3a75ef234..6e38528c0 100644 --- a/include/rocksdb/file_checksum.h +++ b/include/rocksdb/file_checksum.h @@ -18,6 +18,11 @@ namespace ROCKSDB_NAMESPACE { +// The unknown file checksum. +constexpr char kUnknownFileChecksum[] = ""; +// The unknown sst file checksum function name. +constexpr char kUnknownFileChecksumFuncName[] = "Unknown"; + struct FileChecksumGenContext { std::string file_name; }; diff --git a/options/db_options.cc b/options/db_options.cc index 2ce46cad0..2fad9ef17 100644 --- a/options/db_options.cc +++ b/options/db_options.cc @@ -7,7 +7,6 @@ #include -#include "db/version_edit.h" #include "logging/logging.h" #include "options/options_helper.h" #include "port/port.h" @@ -616,9 +615,8 @@ void ImmutableDBOptions::Dump(Logger* log) const { log, " Options.log_readahead_size: %" ROCKSDB_PRIszt, log_readahead_size); ROCKS_LOG_HEADER(log, " Options.file_checksum_gen_factory: %s", - file_checksum_gen_factory - ? file_checksum_gen_factory->Name() - : kUnknownFileChecksumFuncName.c_str()); + file_checksum_gen_factory ? file_checksum_gen_factory->Name() + : kUnknownFileChecksumFuncName); ROCKS_LOG_HEADER(log, " Options.best_efforts_recovery: %d", static_cast(best_efforts_recovery)); } diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index f6f0b95a1..bc1b16ec2 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -1781,7 +1781,7 @@ const char* BlockBasedTableBuilder::GetFileChecksumFuncName() const { if (rep_->file != nullptr) { return rep_->file->GetFileChecksumFuncName(); } else { - return kUnknownFileChecksumFuncName.c_str(); + return kUnknownFileChecksumFuncName; } } diff --git a/table/cuckoo/cuckoo_table_builder.cc b/table/cuckoo/cuckoo_table_builder.cc index 11a081274..7e5d7a7f3 100644 --- a/table/cuckoo/cuckoo_table_builder.cc +++ b/table/cuckoo/cuckoo_table_builder.cc @@ -528,7 +528,7 @@ const char* CuckooTableBuilder::GetFileChecksumFuncName() const { if (file_ != nullptr) { return file_->GetFileChecksumFuncName(); } else { - return kUnknownFileChecksumFuncName.c_str(); + return kUnknownFileChecksumFuncName; } } diff --git a/table/mock_table.h b/table/mock_table.h index 1e77288e2..097809c0c 100644 --- a/table/mock_table.h +++ b/table/mock_table.h @@ -163,7 +163,7 @@ class MockTableBuilder : public TableBuilder { std::string GetFileChecksum() const override { return kUnknownFileChecksum; } // Get file checksum function name const char* GetFileChecksumFuncName() const override { - return kUnknownFileChecksumFuncName.c_str(); + return kUnknownFileChecksumFuncName; } private: diff --git a/table/plain/plain_table_builder.cc b/table/plain/plain_table_builder.cc index 8a1fc5dd1..56cc64547 100644 --- a/table/plain/plain_table_builder.cc +++ b/table/plain/plain_table_builder.cc @@ -313,7 +313,7 @@ const char* PlainTableBuilder::GetFileChecksumFuncName() const { if (file_ != nullptr) { return file_->GetFileChecksumFuncName(); } else { - return kUnknownFileChecksumFuncName.c_str(); + return kUnknownFileChecksumFuncName; } } diff --git a/table/table_test.cc b/table/table_test.cc index e6930eca3..90d032d9c 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -3284,9 +3284,8 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) { f.ResetTableBuilder(std::move(builder)); f.AddKVtoKVMap(1000); f.WriteKVAndFlushTable(); - ASSERT_STREQ(f.GetFileChecksumFuncName(), - kUnknownFileChecksumFuncName.c_str()); - ASSERT_STREQ(f.GetFileChecksum().c_str(), kUnknownFileChecksum.c_str()); + ASSERT_STREQ(f.GetFileChecksumFuncName(), kUnknownFileChecksumFuncName); + ASSERT_STREQ(f.GetFileChecksum().c_str(), kUnknownFileChecksum); } TEST_P(BlockBasedTableTest, Crc32cFileChecksum) { @@ -3430,9 +3429,8 @@ TEST_F(PlainTableTest, NoFileChecksum) { f.ResetTableBuilder(std::move(builder)); f.AddKVtoKVMap(1000); f.WriteKVAndFlushTable(); - ASSERT_STREQ(f.GetFileChecksumFuncName(), - kUnknownFileChecksumFuncName.c_str()); - EXPECT_EQ(f.GetFileChecksum(), kUnknownFileChecksum.c_str()); + ASSERT_STREQ(f.GetFileChecksumFuncName(), kUnknownFileChecksumFuncName); + EXPECT_EQ(f.GetFileChecksum(), kUnknownFileChecksum); } TEST_F(PlainTableTest, Crc32cFileChecksum) {