Prepare for deprecation of Options::access_hint_on_compaction_start (f…

…acebook#11658)

Summary:
**Context/Summary:**
After facebook#11631, file hint is not longer needed for compaction read. Therefore we can deprecate `Options::access_hint_on_compaction_start`. As this is a public API change, we should first mark the relevant APIs (including the Java's) deprecated and remove it in next major release 9.0.

Pull Request resolved: facebook#11658

Test Plan: No code change

Reviewed By: ajkr

Differential Revision: D47997856

Pulled By: hx235

fbshipit-source-id: 16e015ae7728c224b1caef73143aa9915668f4ac

db/db_impl/db_impl_open.cc

@@ -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;
   }
 

db/db_options_test.cc

@@ -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++) {

file/prefetch_test.cc

@@ -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,

include/rocksdb/options.h

@@ -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

java/src/main/java/org/rocksdb/AccessHint.java

@@ -8,6 +8,7 @@
 /**
  * File access pattern once a compaction has started
  */
+@Deprecated
 public enum AccessHint {
   NONE((byte)0x0),
   NORMAL((byte)0x1),

java/src/main/java/org/rocksdb/DBOptions.java

@@ -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_));

java/src/main/java/org/rocksdb/DBOptionsInterface.java

@@ -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.
@@ -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

java/src/main/java/org/rocksdb/Options.java

@@ -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_));

java/src/test/java/org/rocksdb/DBOptionsTest.java

@@ -453,6 +453,7 @@ public void setWriteBufferManagerWithZeroBufferSize() throws RocksDBException {
     }
   }
 
+  @SuppressWarnings("deprecated")
   @Test
   public void accessHintOnCompactionStart() {
     try(final DBOptions opt = new DBOptions()) {

java/src/test/java/org/rocksdb/OptionsTest.java

@@ -699,6 +699,7 @@ public void setWriteBufferManagerWithAllowStall() throws RocksDBException {
     }
   }
 
+  @SuppressWarnings("deprecated")
   @Test
   public void accessHintOnCompactionStart() {
     try (final Options opt = new Options()) {

table/block_based/block_based_table_reader.cc

@@ -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 {

unreleased_history/public_api_changes/mark_dep_access_hint_on_compaction_start.md

@@ -0,0 +1 @@
+Mark `Options::access_hint_on_compaction_start` related APIs as deprecated. See #11631 for alternative behavior.