Clean up VersionBuilder a bit (#6556)

Summary:
The whole point of the pimpl idiom is to hide implementation details.
Internal helper methods like `CheckConsistency`, `CheckConsistencyForDeletes`,
and `MaybeAddFile` do not belong in the public interface of the class.
In addition, the patch switches to `unique_ptr` for the implementation
object instead of using a raw `delete`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6556

Test Plan: `make check`

Reviewed By: riversand963

Differential Revision: D20523568

Pulled By: ltamasi

fbshipit-source-id: 5bbb0ccebd0c47a33b815398c7f9cfe13bd775ac
main
Levi Tamasi 5 years ago committed by Facebook GitHub Bot
parent 217ce20021
commit 442404558a
  1. 16
      db/version_builder.cc
  2. 10
      db/version_builder.h

@ -507,16 +507,7 @@ VersionBuilder::VersionBuilder(const FileOptions& file_options,
Logger* info_log) Logger* info_log)
: rep_(new Rep(file_options, info_log, table_cache, base_vstorage)) {} : rep_(new Rep(file_options, info_log, table_cache, base_vstorage)) {}
VersionBuilder::~VersionBuilder() { delete rep_; } VersionBuilder::~VersionBuilder() = default;
Status VersionBuilder::CheckConsistency(VersionStorageInfo* vstorage) {
return rep_->CheckConsistency(vstorage);
}
Status VersionBuilder::CheckConsistencyForDeletes(VersionEdit* edit,
uint64_t number, int level) {
return rep_->CheckConsistencyForDeletes(edit, number, level);
}
bool VersionBuilder::CheckConsistencyForNumLevels() { bool VersionBuilder::CheckConsistencyForNumLevels() {
return rep_->CheckConsistencyForNumLevels(); return rep_->CheckConsistencyForNumLevels();
@ -537,9 +528,4 @@ Status VersionBuilder::LoadTableHandlers(
is_initial_load, prefix_extractor); is_initial_load, prefix_extractor);
} }
void VersionBuilder::MaybeAddFile(VersionStorageInfo* vstorage, int level,
FileMetaData* f) {
rep_->MaybeAddFile(vstorage, level, f);
}
} // namespace ROCKSDB_NAMESPACE } // namespace ROCKSDB_NAMESPACE

@ -8,6 +8,9 @@
// found in the LICENSE file. See the AUTHORS file for names of contributors. // found in the LICENSE file. See the AUTHORS file for names of contributors.
// //
#pragma once #pragma once
#include <memory>
#include "rocksdb/file_system.h" #include "rocksdb/file_system.h"
#include "rocksdb/slice_transform.h" #include "rocksdb/slice_transform.h"
@ -27,9 +30,7 @@ class VersionBuilder {
VersionBuilder(const FileOptions& file_options, TableCache* table_cache, VersionBuilder(const FileOptions& file_options, TableCache* table_cache,
VersionStorageInfo* base_vstorage, Logger* info_log = nullptr); VersionStorageInfo* base_vstorage, Logger* info_log = nullptr);
~VersionBuilder(); ~VersionBuilder();
Status CheckConsistency(VersionStorageInfo* vstorage);
Status CheckConsistencyForDeletes(VersionEdit* edit, uint64_t number,
int level);
bool CheckConsistencyForNumLevels(); bool CheckConsistencyForNumLevels();
Status Apply(VersionEdit* edit); Status Apply(VersionEdit* edit);
Status SaveTo(VersionStorageInfo* vstorage); Status SaveTo(VersionStorageInfo* vstorage);
@ -37,11 +38,10 @@ class VersionBuilder {
bool prefetch_index_and_filter_in_cache, bool prefetch_index_and_filter_in_cache,
bool is_initial_load, bool is_initial_load,
const SliceTransform* prefix_extractor); const SliceTransform* prefix_extractor);
void MaybeAddFile(VersionStorageInfo* vstorage, int level, FileMetaData* f);
private: private:
class Rep; class Rep;
Rep* rep_; std::unique_ptr<Rep> rep_;
}; };
extern bool NewestFirstBySeqNo(FileMetaData* a, FileMetaData* b); extern bool NewestFirstBySeqNo(FileMetaData* a, FileMetaData* b);

Loading…
Cancel
Save