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
main
Yanqin Jin 5 years ago committed by Facebook GitHub Bot
parent f8c2e5a608
commit 3020df9df5
  1. 4
      db/version_edit.cc
  2. 3
      db/version_edit.h
  3. 2
      file/writable_file_writer.cc
  4. 5
      include/rocksdb/file_checksum.h
  5. 6
      options/db_options.cc
  6. 2
      table/block_based/block_based_table_builder.cc
  7. 2
      table/cuckoo/cuckoo_table_builder.cc
  8. 2
      table/mock_table.h
  9. 2
      table/plain/plain_table_builder.cc
  10. 10
      table/table_test.cc

@ -18,10 +18,6 @@
#include "util/string_util.h" #include "util/string_util.h"
namespace ROCKSDB_NAMESPACE { namespace ROCKSDB_NAMESPACE {
// The unknown file checksum.
const std::string kUnknownFileChecksum("");
// The unknown sst file checksum function name.
const std::string kUnknownFileChecksumFuncName("Unknown");
namespace { namespace {

@ -29,9 +29,6 @@ constexpr uint64_t kFileNumberMask = 0x3FFFFFFFFFFFFFFF;
constexpr uint64_t kUnknownOldestAncesterTime = 0; constexpr uint64_t kUnknownOldestAncesterTime = 0;
constexpr uint64_t kUnknownFileCreationTime = 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); extern uint64_t PackFileNumberAndPathId(uint64_t number, uint64_t path_id);
// A copyable structure contains information needed to read data from an SST // A copyable structure contains information needed to read data from an SST

@ -236,7 +236,7 @@ const char* WritableFileWriter::GetFileChecksumFuncName() const {
if (checksum_generator_ != nullptr) { if (checksum_generator_ != nullptr) {
return checksum_generator_->Name(); return checksum_generator_->Name();
} else { } else {
return kUnknownFileChecksumFuncName.c_str(); return kUnknownFileChecksumFuncName;
} }
} }

@ -18,6 +18,11 @@
namespace ROCKSDB_NAMESPACE { namespace ROCKSDB_NAMESPACE {
// The unknown file checksum.
constexpr char kUnknownFileChecksum[] = "";
// The unknown sst file checksum function name.
constexpr char kUnknownFileChecksumFuncName[] = "Unknown";
struct FileChecksumGenContext { struct FileChecksumGenContext {
std::string file_name; std::string file_name;
}; };

@ -7,7 +7,6 @@
#include <cinttypes> #include <cinttypes>
#include "db/version_edit.h"
#include "logging/logging.h" #include "logging/logging.h"
#include "options/options_helper.h" #include "options/options_helper.h"
#include "port/port.h" #include "port/port.h"
@ -616,9 +615,8 @@ void ImmutableDBOptions::Dump(Logger* log) const {
log, " Options.log_readahead_size: %" ROCKSDB_PRIszt, log, " Options.log_readahead_size: %" ROCKSDB_PRIszt,
log_readahead_size); log_readahead_size);
ROCKS_LOG_HEADER(log, " Options.file_checksum_gen_factory: %s", ROCKS_LOG_HEADER(log, " Options.file_checksum_gen_factory: %s",
file_checksum_gen_factory file_checksum_gen_factory ? file_checksum_gen_factory->Name()
? file_checksum_gen_factory->Name() : kUnknownFileChecksumFuncName);
: kUnknownFileChecksumFuncName.c_str());
ROCKS_LOG_HEADER(log, " Options.best_efforts_recovery: %d", ROCKS_LOG_HEADER(log, " Options.best_efforts_recovery: %d",
static_cast<int>(best_efforts_recovery)); static_cast<int>(best_efforts_recovery));
} }

@ -1781,7 +1781,7 @@ const char* BlockBasedTableBuilder::GetFileChecksumFuncName() const {
if (rep_->file != nullptr) { if (rep_->file != nullptr) {
return rep_->file->GetFileChecksumFuncName(); return rep_->file->GetFileChecksumFuncName();
} else { } else {
return kUnknownFileChecksumFuncName.c_str(); return kUnknownFileChecksumFuncName;
} }
} }

@ -528,7 +528,7 @@ const char* CuckooTableBuilder::GetFileChecksumFuncName() const {
if (file_ != nullptr) { if (file_ != nullptr) {
return file_->GetFileChecksumFuncName(); return file_->GetFileChecksumFuncName();
} else { } else {
return kUnknownFileChecksumFuncName.c_str(); return kUnknownFileChecksumFuncName;
} }
} }

@ -163,7 +163,7 @@ class MockTableBuilder : public TableBuilder {
std::string GetFileChecksum() const override { return kUnknownFileChecksum; } std::string GetFileChecksum() const override { return kUnknownFileChecksum; }
// Get file checksum function name // Get file checksum function name
const char* GetFileChecksumFuncName() const override { const char* GetFileChecksumFuncName() const override {
return kUnknownFileChecksumFuncName.c_str(); return kUnknownFileChecksumFuncName;
} }
private: private:

@ -313,7 +313,7 @@ const char* PlainTableBuilder::GetFileChecksumFuncName() const {
if (file_ != nullptr) { if (file_ != nullptr) {
return file_->GetFileChecksumFuncName(); return file_->GetFileChecksumFuncName();
} else { } else {
return kUnknownFileChecksumFuncName.c_str(); return kUnknownFileChecksumFuncName;
} }
} }

@ -3284,9 +3284,8 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) {
f.ResetTableBuilder(std::move(builder)); f.ResetTableBuilder(std::move(builder));
f.AddKVtoKVMap(1000); f.AddKVtoKVMap(1000);
f.WriteKVAndFlushTable(); f.WriteKVAndFlushTable();
ASSERT_STREQ(f.GetFileChecksumFuncName(), ASSERT_STREQ(f.GetFileChecksumFuncName(), kUnknownFileChecksumFuncName);
kUnknownFileChecksumFuncName.c_str()); ASSERT_STREQ(f.GetFileChecksum().c_str(), kUnknownFileChecksum);
ASSERT_STREQ(f.GetFileChecksum().c_str(), kUnknownFileChecksum.c_str());
} }
TEST_P(BlockBasedTableTest, Crc32cFileChecksum) { TEST_P(BlockBasedTableTest, Crc32cFileChecksum) {
@ -3430,9 +3429,8 @@ TEST_F(PlainTableTest, NoFileChecksum) {
f.ResetTableBuilder(std::move(builder)); f.ResetTableBuilder(std::move(builder));
f.AddKVtoKVMap(1000); f.AddKVtoKVMap(1000);
f.WriteKVAndFlushTable(); f.WriteKVAndFlushTable();
ASSERT_STREQ(f.GetFileChecksumFuncName(), ASSERT_STREQ(f.GetFileChecksumFuncName(), kUnknownFileChecksumFuncName);
kUnknownFileChecksumFuncName.c_str()); EXPECT_EQ(f.GetFileChecksum(), kUnknownFileChecksum);
EXPECT_EQ(f.GetFileChecksum(), kUnknownFileChecksum.c_str());
} }
TEST_F(PlainTableTest, Crc32cFileChecksum) { TEST_F(PlainTableTest, Crc32cFileChecksum) {

Loading…
Cancel
Save