From 0cc9e98bbb6cd8b1c41ea492c4d2bd40ca25c5c9 Mon Sep 17 00:00:00 2001
From: Levi Tamasi <ltamasi@fb.com>
Date: Mon, 8 Aug 2022 08:26:33 -0700
Subject: [PATCH] Respect fill_cache when reading blobs in DBIter (#10492)

Summary:
Similarly to https://github.com/facebook/rocksdb/pull/10457, we now have
to explicitly set the `fill_cache` read option when reading blobs in
`DBIter` to prevent the cache from getting polluted by queries with
`fill_cache` set to false. (Before we added support for a blob cache,
the setting had not made any difference either way.)

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10492

Test Plan: `make check`

Reviewed By: akankshamahajan15

Differential Revision: D38476121

Pulled By: ltamasi

fbshipit-source-id: ea5c5e252f83e4a4e2c74156b37d40308d7e0c80
---
 db/blob/db_blob_basic_test.cc | 7 +++++++
 db/db_iter.cc                 | 2 ++
 db/db_iter.h                  | 1 +
 3 files changed, 10 insertions(+)

diff --git a/db/blob/db_blob_basic_test.cc b/db/blob/db_blob_basic_test.cc
index 18ab1ef7d..95ed18253 100644
--- a/db/blob/db_blob_basic_test.cc
+++ b/db/blob/db_blob_basic_test.cc
@@ -135,6 +135,8 @@ TEST_F(DBBlobBasicTest, IterateBlobsFromCache) {
   block_based_options.cache_index_and_filter_blocks = true;
   options.table_factory.reset(NewBlockBasedTableFactory(block_based_options));
 
+  options.statistics = CreateDBStatistics();
+
   Reopen(options);
 
   int num_blobs = 5;
@@ -165,6 +167,7 @@ TEST_F(DBBlobBasicTest, IterateBlobsFromCache) {
       ++i;
     }
     ASSERT_EQ(i, num_blobs);
+    ASSERT_EQ(options.statistics->getAndResetTickerCount(BLOB_DB_CACHE_ADD), 0);
   }
 
   {
@@ -180,6 +183,7 @@ TEST_F(DBBlobBasicTest, IterateBlobsFromCache) {
     iter->SeekToFirst();
     ASSERT_NOK(iter->status());
     ASSERT_FALSE(iter->Valid());
+    ASSERT_EQ(options.statistics->getAndResetTickerCount(BLOB_DB_CACHE_ADD), 0);
   }
 
   {
@@ -198,6 +202,8 @@ TEST_F(DBBlobBasicTest, IterateBlobsFromCache) {
       ++i;
     }
     ASSERT_EQ(i, num_blobs);
+    ASSERT_EQ(options.statistics->getAndResetTickerCount(BLOB_DB_CACHE_ADD),
+              num_blobs);
   }
 
   {
@@ -217,6 +223,7 @@ TEST_F(DBBlobBasicTest, IterateBlobsFromCache) {
       ++i;
     }
     ASSERT_EQ(i, num_blobs);
+    ASSERT_EQ(options.statistics->getAndResetTickerCount(BLOB_DB_CACHE_ADD), 0);
   }
 }
 
diff --git a/db/db_iter.cc b/db/db_iter.cc
index 7f2cb8f49..371c20d32 100644
--- a/db/db_iter.cc
+++ b/db/db_iter.cc
@@ -71,6 +71,7 @@ DBIter::DBIter(Env* _env, const ReadOptions& read_options,
                                      read_options.total_order_seek ||
                                      read_options.auto_prefix_mode),
       read_tier_(read_options.read_tier),
+      fill_cache_(read_options.fill_cache),
       verify_checksums_(read_options.verify_checksums),
       expose_blob_index_(expose_blob_index),
       is_blob_(false),
@@ -189,6 +190,7 @@ bool DBIter::SetBlobValueIfNeeded(const Slice& user_key,
   // avoid having to copy options back and forth.
   ReadOptions read_options;
   read_options.read_tier = read_tier_;
+  read_options.fill_cache = fill_cache_;
   read_options.verify_checksums = verify_checksums_;
 
   constexpr FilePrefetchBuffer* prefetch_buffer = nullptr;
diff --git a/db/db_iter.h b/db/db_iter.h
index 8a4ee3792..1847060dd 100644
--- a/db/db_iter.h
+++ b/db/db_iter.h
@@ -356,6 +356,7 @@ class DBIter final : public Iterator {
   // prefix_extractor_ must be non-NULL if the value is false.
   const bool expect_total_order_inner_iter_;
   ReadTier read_tier_;
+  bool fill_cache_;
   bool verify_checksums_;
   // Whether the iterator is allowed to expose blob references. Set to true when
   // the stacked BlobDB implementation is used, false otherwise.