Skip to content

Commit

Permalink
Make ImmutableOptions struct that inherits from ImmutableCFOptions an…
Browse files Browse the repository at this point in the history 
…d ImmutableDBOptions (facebook#8262)

Summary:
The ImmutableCFOptions contained a bunch of fields that belonged to the ImmutableDBOptions.  This change cleans that up by introducing an ImmutableOptions struct.  Following the pattern of Options struct, this class inherits from the DB and CFOption structs (of the Immutable form).

Only one structural change (the ImmutableCFOptions::fs was changed to a shared_ptr from a raw one) is in this PR.  All of the other changes involve moving the member variables from the ImmutableCFOptions into the ImmutableOptions and changing member variables or function parameters as required for compilation purposes.

Follow-on PRs may do a further clean-up of the code, such as renaming variables (such as "ImmutableOptions cf_options") and potentially eliminating un-needed function parameters (there is no longer a need to pass both an ImmutableDBOptions and an ImmutableOptions to a function).

Pull Request resolved: facebook#8262

Reviewed By: pdillinger

Differential Revision: D28226540

Pulled By: mrambacher

fbshipit-source-id: 18ae71eadc879dedbe38b1eb8e6f9ff5c7147dbf
  • Loading branch information
@mrambacher @facebook-github-bot
mrambacher authored and facebook-github-bot committed May 5, 2021
1 parent 0f42e50 commit 8948dc8
Show file tree
Hide file tree
Showing 87 changed files with 453 additions and 571 deletions.
4 changes: 2 additions & 2 deletions db/arena_wrapped_db_iter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Status ArenaWrappedDBIter::GetProperty(std::string prop_name,

void ArenaWrappedDBIter::Init(
Env* env, const ReadOptions& read_options,
const ImmutableCFOptions& cf_options,
const ImmutableOptions& cf_options,
const MutableCFOptions& mutable_cf_options, const Version* version,
const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iteration,
uint64_t version_number, ReadCallback* read_callback, DBImpl* db_impl,
Expand Down Expand Up @@ -89,7 +89,7 @@ Status ArenaWrappedDBIter::Refresh() {

ArenaWrappedDBIter* NewArenaWrappedDbIterator(
Env* env, const ReadOptions& read_options,
const ImmutableCFOptions& cf_options,
const ImmutableOptions& cf_options,
const MutableCFOptions& mutable_cf_options, const Version* version,
const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,
uint64_t version_number, ReadCallback* read_callback, DBImpl* db_impl,
Expand Down
4 changes: 2 additions & 2 deletions db/arena_wrapped_db_iter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class ArenaWrappedDBIter : public Iterator {
Status Refresh() override;

void Init(Env* env, const ReadOptions& read_options,
const ImmutableCFOptions& cf_options,
const ImmutableOptions& cf_options,
const MutableCFOptions& mutable_cf_options, const Version* version,
const SequenceNumber& sequence,
uint64_t max_sequential_skip_in_iterations, uint64_t version_number,
Expand Down Expand Up @@ -106,7 +106,7 @@ class ArenaWrappedDBIter : public Iterator {
// be supported.
extern ArenaWrappedDBIter* NewArenaWrappedDbIterator(
Env* env, const ReadOptions& read_options,
const ImmutableCFOptions& cf_options,
const ImmutableOptions& cf_options,
const MutableCFOptions& mutable_cf_options, const Version* version,
const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,
uint64_t version_number, ReadCallback* read_callback,
Expand Down
4 changes: 2 additions & 2 deletions db/blob/blob_file_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace ROCKSDB_NAMESPACE {

BlobFileBuilder::BlobFileBuilder(
VersionSet* versions, FileSystem* fs,
const ImmutableCFOptions* immutable_cf_options,
const ImmutableOptions* immutable_cf_options,
const MutableCFOptions* mutable_cf_options, const FileOptions* file_options,
int job_id, uint32_t column_family_id,
const std::string& column_family_name, Env::IOPriority io_priority,
Expand All @@ -46,7 +46,7 @@ BlobFileBuilder::BlobFileBuilder(

BlobFileBuilder::BlobFileBuilder(
std::function<uint64_t()> file_number_generator, FileSystem* fs,
const ImmutableCFOptions* immutable_cf_options,
const ImmutableOptions* immutable_cf_options,
const MutableCFOptions* mutable_cf_options, const FileOptions* file_options,
int job_id, uint32_t column_family_id,
const std::string& column_family_name, Env::IOPriority io_priority,
Expand Down
9 changes: 4 additions & 5 deletions db/blob/blob_file_builder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace ROCKSDB_NAMESPACE {
class VersionSet;
class FileSystem;
class SystemClock;
struct ImmutableCFOptions;
struct ImmutableOptions;
struct MutableCFOptions;
struct FileOptions;
class BlobFileAddition;
Expand All @@ -32,7 +32,7 @@ class BlobFileCompletionCallback;
class BlobFileBuilder {
public:
BlobFileBuilder(VersionSet* versions, FileSystem* fs,
const ImmutableCFOptions* immutable_cf_options,
const ImmutableOptions* immutable_options,
const MutableCFOptions* mutable_cf_options,
const FileOptions* file_options, int job_id,
uint32_t column_family_id,
Expand All @@ -45,8 +45,7 @@ class BlobFileBuilder {
std::vector<BlobFileAddition>* blob_file_additions);

BlobFileBuilder(std::function<uint64_t()> file_number_generator,
FileSystem* fs,
const ImmutableCFOptions* immutable_cf_options,
FileSystem* fs, const ImmutableOptions* immutable_options,
const MutableCFOptions* mutable_cf_options,
const FileOptions* file_options, int job_id,
uint32_t column_family_id,
Expand Down Expand Up @@ -78,7 +77,7 @@ class BlobFileBuilder {

std::function<uint64_t()> file_number_generator_;
FileSystem* fs_;
const ImmutableCFOptions* immutable_cf_options_;
const ImmutableOptions* immutable_cf_options_;
uint64_t min_blob_size_;
uint64_t blob_file_size_;
CompressionType blob_compression_type_;
Expand Down
14 changes: 7 additions & 7 deletions db/blob/blob_file_builder_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ TEST_F(BlobFileBuilderTest, BuildAndCheckOneFile) {
options.enable_blob_files = true;
options.env = &mock_env_;

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down Expand Up @@ -214,7 +214,7 @@ TEST_F(BlobFileBuilderTest, BuildAndCheckMultipleFiles) {
options.blob_file_size = value_size;
options.env = &mock_env_;

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down Expand Up @@ -301,7 +301,7 @@ TEST_F(BlobFileBuilderTest, InlinedValues) {
options.min_blob_size = 1024;
options.env = &mock_env_;

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down Expand Up @@ -355,7 +355,7 @@ TEST_F(BlobFileBuilderTest, Compression) {
options.blob_compression_type = kSnappyCompression;
options.env = &mock_env_;

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down Expand Up @@ -438,7 +438,7 @@ TEST_F(BlobFileBuilderTest, CompressionError) {
options.enable_blob_files = true;
options.blob_compression_type = kSnappyCompression;
options.env = &mock_env_;
ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down Expand Up @@ -517,7 +517,7 @@ TEST_F(BlobFileBuilderTest, Checksum) {
std::make_shared<DummyFileChecksumGenFactory>();
options.env = &mock_env_;

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down Expand Up @@ -614,7 +614,7 @@ TEST_P(BlobFileBuilderIOErrorTest, IOError) {
options.blob_file_size = value_size;
options.env = &mock_env_;

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
MutableCFOptions mutable_cf_options(options);

constexpr int job_id = 1;
Expand Down
4 changes: 2 additions & 2 deletions db/blob/blob_file_cache.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@
namespace ROCKSDB_NAMESPACE {

BlobFileCache::BlobFileCache(Cache* cache,
const ImmutableCFOptions* immutable_cf_options,
const ImmutableOptions* immutable_options,
const FileOptions* file_options,
uint32_t column_family_id,
HistogramImpl* blob_file_read_hist,
const std::shared_ptr<IOTracer>& io_tracer)
: cache_(cache),
mutex_(kNumberOfMutexStripes, kGetSliceNPHash64UnseededFnPtr),
immutable_cf_options_(immutable_cf_options),
immutable_cf_options_(immutable_options),
file_options_(file_options),
column_family_id_(column_family_id),
blob_file_read_hist_(blob_file_read_hist),
Expand Down
6 changes: 3 additions & 3 deletions db/blob/blob_file_cache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
namespace ROCKSDB_NAMESPACE {

class Cache;
struct ImmutableCFOptions;
struct ImmutableOptions;
struct FileOptions;
class HistogramImpl;
class Status;
Expand All @@ -24,7 +24,7 @@ class IOTracer;

class BlobFileCache {
public:
BlobFileCache(Cache* cache, const ImmutableCFOptions* immutable_cf_options,
BlobFileCache(Cache* cache, const ImmutableOptions* immutable_options,
const FileOptions* file_options, uint32_t column_family_id,
HistogramImpl* blob_file_read_hist,
const std::shared_ptr<IOTracer>& io_tracer);
Expand All @@ -40,7 +40,7 @@ class BlobFileCache {
// Note: mutex_ below is used to guard against multiple threads racing to open
// the same file.
Striped<port::Mutex, Slice> mutex_;
const ImmutableCFOptions* immutable_cf_options_;
const ImmutableOptions* immutable_cf_options_;
const FileOptions* file_options_;
uint32_t column_family_id_;
HistogramImpl* blob_file_read_hist_;
Expand Down
14 changes: 7 additions & 7 deletions db/blob/blob_file_cache_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ namespace {

// Creates a test blob file with a single blob in it.
void WriteBlobFile(uint32_t column_family_id,
const ImmutableCFOptions& immutable_cf_options,
const ImmutableOptions& immutable_cf_options,
uint64_t blob_file_number) {
assert(!immutable_cf_options.cf_paths.empty());

const std::string blob_file_path = BlobFileName(
immutable_cf_options.cf_paths.front().path, blob_file_number);

std::unique_ptr<FSWritableFile> file;
ASSERT_OK(NewWritableFile(immutable_cf_options.fs, blob_file_path, &file,
FileOptions()));
ASSERT_OK(NewWritableFile(immutable_cf_options.fs.get(), blob_file_path,
&file, FileOptions()));

std::unique_ptr<WritableFileWriter> file_writer(
new WritableFileWriter(std::move(file), blob_file_path, FileOptions(),
Expand Down Expand Up @@ -100,7 +100,7 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader) {
options.enable_blob_files = true;

constexpr uint32_t column_family_id = 1;
ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
constexpr uint64_t blob_file_number = 123;

WriteBlobFile(column_family_id, immutable_cf_options, blob_file_number);
Expand Down Expand Up @@ -145,7 +145,7 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader_Race) {
options.enable_blob_files = true;

constexpr uint32_t column_family_id = 1;
ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
constexpr uint64_t blob_file_number = 123;

WriteBlobFile(column_family_id, immutable_cf_options, blob_file_number);
Expand Down Expand Up @@ -199,7 +199,7 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader_IOError) {
constexpr size_t capacity = 10;
std::shared_ptr<Cache> backing_cache = NewLRUCache(capacity);

ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
FileOptions file_options;
constexpr uint32_t column_family_id = 1;
constexpr HistogramImpl* blob_file_read_hist = nullptr;
Expand Down Expand Up @@ -231,7 +231,7 @@ TEST_F(BlobFileCacheTest, GetBlobFileReader_CacheFull) {
options.enable_blob_files = true;

constexpr uint32_t column_family_id = 1;
ImmutableCFOptions immutable_cf_options(options);
ImmutableOptions immutable_cf_options(options);
constexpr uint64_t blob_file_number = 123;

WriteBlobFile(column_family_id, immutable_cf_options, blob_file_number);
Expand Down
12 changes: 6 additions & 6 deletions db/blob/blob_file_reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
namespace ROCKSDB_NAMESPACE {

Status BlobFileReader::Create(
const ImmutableCFOptions& immutable_cf_options,
const ImmutableOptions& immutable_cf_options,
const FileOptions& file_options, uint32_t column_family_id,
HistogramImpl* blob_file_read_hist, uint64_t blob_file_number,
const std::shared_ptr<IOTracer>& io_tracer,
Expand Down Expand Up @@ -67,10 +67,10 @@ Status BlobFileReader::Create(
}

Status BlobFileReader::OpenFile(
const ImmutableCFOptions& immutable_cf_options,
const FileOptions& file_opts, HistogramImpl* blob_file_read_hist,
uint64_t blob_file_number, const std::shared_ptr<IOTracer>& io_tracer,
uint64_t* file_size, std::unique_ptr<RandomAccessFileReader>* file_reader) {
const ImmutableOptions& immutable_cf_options, const FileOptions& file_opts,
HistogramImpl* blob_file_read_hist, uint64_t blob_file_number,
const std::shared_ptr<IOTracer>& io_tracer, uint64_t* file_size,
std::unique_ptr<RandomAccessFileReader>* file_reader) {
assert(file_size);
assert(file_reader);

Expand All @@ -80,7 +80,7 @@ Status BlobFileReader::OpenFile(
const std::string blob_file_path =
BlobFileName(cf_paths.front().path, blob_file_number);

FileSystem* const fs = immutable_cf_options.fs;
FileSystem* const fs = immutable_cf_options.fs.get();
assert(fs);

constexpr IODebugContext* dbg = nullptr;
Expand Down
6 changes: 3 additions & 3 deletions db/blob/blob_file_reader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
namespace ROCKSDB_NAMESPACE {

class Status;
struct ImmutableCFOptions;
struct ImmutableOptions;
struct FileOptions;
class HistogramImpl;
struct ReadOptions;
Expand All @@ -24,7 +24,7 @@ class PinnableSlice;

class BlobFileReader {
public:
static Status Create(const ImmutableCFOptions& immutable_cf_options,
static Status Create(const ImmutableOptions& immutable_options,
const FileOptions& file_options,
uint32_t column_family_id,
HistogramImpl* blob_file_read_hist,
Expand All @@ -46,7 +46,7 @@ class BlobFileReader {
BlobFileReader(std::unique_ptr<RandomAccessFileReader>&& file_reader,
uint64_t file_size, CompressionType compression_type);

static Status OpenFile(const ImmutableCFOptions& immutable_cf_options,
static Status OpenFile(const ImmutableOptions& immutable_options,
const FileOptions& file_opts,
HistogramImpl* blob_file_read_hist,
uint64_t blob_file_number,
Expand Down
Loading

0 comments on commit 8948dc8

Please sign in to comment.