Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare for deprecation of Options::access_hint_on_compaction_start #11658

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions db/db_impl/db_impl_open.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,10 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src,
result.wal_dir = result.wal_dir.substr(0, result.wal_dir.size() - 1);
}

if (result.use_direct_reads && result.compaction_readahead_size == 0) {
TEST_SYNC_POINT_CALLBACK("SanitizeOptions:direct_io", nullptr);
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;
}

Expand Down
3 changes: 2 additions & 1 deletion db/db_options_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,8 @@ TEST_F(DBOptionsTest, CompactionReadaheadSizeChange) {
const std::string kValue(1024, 'v');
Reopen(options);

ASSERT_EQ(0, dbfull()->GetDBOptions().compaction_readahead_size);
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++) {
Expand Down
13 changes: 3 additions & 10 deletions file/prefetch_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,16 +238,9 @@ TEST_P(PrefetchTest, Basic) {
fs->ClearPrefetchCount();
} else {
ASSERT_FALSE(fs->IsPrefetchCalled());
if (use_direct_io) {
// To rule out false positive by the SST file tail prefetch during
// compaction output verification
ASSERT_GT(buff_prefetch_count, 1);
} else {
// In buffered IO, compaction readahead size is 0, leading to no prefetch
// during compaction input read
ASSERT_EQ(buff_prefetch_count, 1);
}

// To rule out false positive by the SST file tail prefetch during
// compaction output verification
ASSERT_GT(buff_prefetch_count, 1);
buff_prefetch_count = 0;

ASSERT_GT(cur_table_open_prefetch_tail_read.count,
Expand Down
3 changes: 3 additions & 0 deletions include/rocksdb/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,9 @@ struct DBOptions {
// Default: null
std::shared_ptr<WriteBufferManager> write_buffer_manager = nullptr;

// DEPRECATED
// This flag has no effect on the behavior of compaction and we plan to delete
// it in the future.
// Specify the file access pattern once a compaction is started.
// It will be applied to all input files of a compaction.
// Default: NORMAL
Expand Down
1 change: 1 addition & 0 deletions java/src/main/java/org/rocksdb/AccessHint.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
/**
* File access pattern once a compaction has started
*/
@Deprecated
public enum AccessHint {
NONE((byte)0x0),
NORMAL((byte)0x1),
Expand Down
2 changes: 2 additions & 0 deletions java/src/main/java/org/rocksdb/DBOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -752,13 +752,15 @@ public long dbWriteBufferSize() {
}

@Override
@Deprecated
public DBOptions setAccessHintOnCompactionStart(final AccessHint accessHint) {
assert(isOwningHandle());
setAccessHintOnCompactionStart(nativeHandle_, accessHint.getValue());
return this;
}

@Override
@Deprecated
public AccessHint accessHintOnCompactionStart() {
assert(isOwningHandle());
return AccessHint.getAccessHint(accessHintOnCompactionStart(nativeHandle_));
Expand Down
4 changes: 2 additions & 2 deletions java/src/main/java/org/rocksdb/DBOptionsInterface.java
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ public interface DBOptionsInterface<T extends DBOptionsInterface<T>> {
*
* @return the reference to the current options.
*/
T setAccessHintOnCompactionStart(final AccessHint accessHint);
@Deprecated T setAccessHintOnCompactionStart(final AccessHint accessHint);

/**
* Specify the file access pattern once a compaction is started.
Expand All @@ -945,7 +945,7 @@ public interface DBOptionsInterface<T extends DBOptionsInterface<T>> {
*
* @return The access hint
*/
AccessHint accessHintOnCompactionStart();
@Deprecated AccessHint accessHintOnCompactionStart();

/**
* This is a maximum buffer size that is used by WinMmapReadableFile in
Expand Down
2 changes: 2 additions & 0 deletions java/src/main/java/org/rocksdb/Options.java
Original file line number Diff line number Diff line change
Expand Up @@ -840,13 +840,15 @@ public long dbWriteBufferSize() {
}

@Override
@Deprecated
public Options setAccessHintOnCompactionStart(final AccessHint accessHint) {
assert(isOwningHandle());
setAccessHintOnCompactionStart(nativeHandle_, accessHint.getValue());
return this;
}

@Override
@Deprecated
public AccessHint accessHintOnCompactionStart() {
assert(isOwningHandle());
return AccessHint.getAccessHint(accessHintOnCompactionStart(nativeHandle_));
Expand Down
1 change: 1 addition & 0 deletions java/src/test/java/org/rocksdb/DBOptionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ public void setWriteBufferManagerWithZeroBufferSize() throws RocksDBException {
}
}

@SuppressWarnings("deprecated")
@Test
public void accessHintOnCompactionStart() {
try(final DBOptions opt = new DBOptions()) {
Expand Down
1 change: 1 addition & 0 deletions java/src/test/java/org/rocksdb/OptionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,7 @@ public void setWriteBufferManagerWithAllowStall() throws RocksDBException {
}
}

@SuppressWarnings("deprecated")
@Test
public void accessHintOnCompactionStart() {
try (final Options opt = new Options()) {
Expand Down
18 changes: 1 addition & 17 deletions table/block_based/block_based_table_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1215,23 +1215,7 @@ Status BlockBasedTable::PrefetchIndexAndFilterBlocks(
return s;
}

void BlockBasedTable::SetupForCompaction() {
switch (rep_->ioptions.access_hint_on_compaction_start) {
case Options::NONE:
break;
case Options::NORMAL:
rep_->file->file()->Hint(FSRandomAccessFile::kNormal);
break;
case Options::SEQUENTIAL:
rep_->file->file()->Hint(FSRandomAccessFile::kSequential);
break;
case Options::WILLNEED:
rep_->file->file()->Hint(FSRandomAccessFile::kWillNeed);
break;
default:
assert(false);
}
}
void BlockBasedTable::SetupForCompaction() {}

std::shared_ptr<const TableProperties> BlockBasedTable::GetTableProperties()
const {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Mark `Options::access_hint_on_compaction_start` related APIs as deprecated. See #11631 for alternative behavior.
Loading