Skip to content

Commit

Permalink
Change default value of compaction_readahead_size
Browse files Browse the repository at this point in the history
  • Loading branch information
hx235 committed Aug 29, 2023
1 parent f36394f commit 39f15a5
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 38 deletions.
7 changes: 1 addition & 6 deletions db/db_compaction_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6025,23 +6025,18 @@ TEST_P(DBCompactionDirectIOTest, DirectIO) {
options.use_direct_io_for_flush_and_compaction = GetParam();
options.env = MockEnv::Create(Env::Default());
Reopen(options);
bool readahead = false;
SyncPoint::GetInstance()->SetCallBack(
"CompactionJob::OpenCompactionOutputFile", [&](void* arg) {
bool* use_direct_writes = static_cast<bool*>(arg);
ASSERT_EQ(*use_direct_writes,
options.use_direct_io_for_flush_and_compaction);
});
if (options.use_direct_io_for_flush_and_compaction) {
SyncPoint::GetInstance()->SetCallBack(
"SanitizeOptions:direct_io", [&](void* /*arg*/) { readahead = true; });
}
SyncPoint::GetInstance()->EnableProcessing();
CreateAndReopenWithCF({"pikachu"}, options);
MakeTables(3, "p", "q", 1);
ASSERT_EQ("1,1,1", FilesPerLevel(1));
Compact(1, "p", "q");
ASSERT_EQ(readahead, options.use_direct_reads);
ASSERT_EQ(false, options.use_direct_reads);
ASSERT_EQ("0,0,1", FilesPerLevel(1));
Destroy(options);
delete options.env;
Expand Down
7 changes: 0 additions & 7 deletions db/db_impl/db_impl_open.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,6 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src,
result.wal_dir = result.wal_dir.substr(0, result.wal_dir.size() - 1);
}

if (result.compaction_readahead_size == 0) {
if (result.use_direct_reads) {
TEST_SYNC_POINT_CALLBACK("SanitizeOptions:direct_io", nullptr);
}
result.compaction_readahead_size = 1024 * 1024 * 2;
}

// Force flush on DB open if 2PC is enabled, since with 2PC we have no
// guarantee that consecutive log files have consecutive sequence id, which
// make recovery complicated.
Expand Down
51 changes: 29 additions & 22 deletions db/db_options_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1034,30 +1034,37 @@ TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {
}

TEST_F(DBOptionsTest, CompactionReadaheadSizeChange) {
SpecialEnv env(env_);
Options options;
options.env = &env;

options.compaction_readahead_size = 0;
options.level0_file_num_compaction_trigger = 2;
const std::string kValue(1024, 'v');
Reopen(options);
for (bool use_direct_reads : {true, false}) {
SpecialEnv env(env_);
Options options;
options.env = &env;

options.use_direct_reads = use_direct_reads;
options.level0_file_num_compaction_trigger = 2;
const std::string kValue(1024, 'v');
Status s = TryReopen(options);
if (use_direct_reads && (s.IsNotSupported() || s.IsInvalidArgument())) {
continue;
} else {
ASSERT_OK(s);
}

ASSERT_EQ(1024 * 1024 * 2,
dbfull()->GetDBOptions().compaction_readahead_size);
ASSERT_OK(dbfull()->SetDBOptions({{"compaction_readahead_size", "256"}}));
ASSERT_EQ(256, dbfull()->GetDBOptions().compaction_readahead_size);
for (int i = 0; i < 1024; i++) {
ASSERT_OK(Put(Key(i), kValue));
}
ASSERT_OK(Flush());
for (int i = 0; i < 1024 * 2; i++) {
ASSERT_OK(Put(Key(i), kValue));
ASSERT_EQ(1024 * 1024 * 2,
dbfull()->GetDBOptions().compaction_readahead_size);
ASSERT_OK(dbfull()->SetDBOptions({{"compaction_readahead_size", "256"}}));
ASSERT_EQ(256, dbfull()->GetDBOptions().compaction_readahead_size);
for (int i = 0; i < 1024; i++) {
ASSERT_OK(Put(Key(i), kValue));
}
ASSERT_OK(Flush());
for (int i = 0; i < 1024 * 2; i++) {
ASSERT_OK(Put(Key(i), kValue));
}
ASSERT_OK(Flush());
ASSERT_OK(dbfull()->TEST_WaitForCompact());
ASSERT_EQ(256, env_->compaction_readahead_size_);
Close();
}
ASSERT_OK(Flush());
ASSERT_OK(dbfull()->TEST_WaitForCompact());
ASSERT_EQ(256, env_->compaction_readahead_size_);
Close();
}

TEST_F(DBOptionsTest, FIFOTtlBackwardCompatible) {
Expand Down
5 changes: 2 additions & 3 deletions include/rocksdb/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -952,16 +952,15 @@ struct DBOptions {
AccessHint access_hint_on_compaction_start = NORMAL;

// The size RocksDB uses to perform readahead during compaction read.
// If set zero, RocksDB will sanitize it to be 2MB during db open.
// If you're
// running RocksDB on spinning disks, you should set this to at least 2MB.
// That way RocksDB's compaction is doing sequential instead of random reads.
//
//
// Default: 0
// Default: 2MB
//
// Dynamically changeable through SetDBOptions() API.
size_t compaction_readahead_size = 0;
size_t compaction_readahead_size = 2 * 1024 * 1024;

// This is a maximum buffer size that is used by WinMmapReadableFile in
// unbuffered disk I/O mode. We need to maintain an aligned buffer for
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
As `Options::access_hint_on_compaction_start` is made to have no effect since #11658, under non-direct IO, setting `Options::compaction_readahead_size` to 0 may regress compaction read performance, because it will not have the readahead issued by Kernel, which is the behavior prior to #11658, nor the intended replacement of Kernel-issued readahead by RocksDB-issued readahead introduced in #11631 now requiring `Options::compaction_readahead_size > 0`
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`Options::compaction_readahead_size` 's default value is changed from 0 to 2MB.

0 comments on commit 39f15a5

Please sign in to comment.