From 244e6f2002dda2959bc5b15f42dd16d8d6984eb3 Mon Sep 17 00:00:00 2001
From: Maysam Yabandeh <myabandeh@fb.com>
Date: Thu, 22 Aug 2019 08:47:36 -0700
Subject: [PATCH] Refactor MultiGet names in BlockBasedTable (#5726)

Summary:
To improve code readability, since RetrieveBlock already calls MaybeReadBlockAndLoadToCache, we avoid name similarity of the functions that call RetrieveBlock with MaybeReadBlockAndLoadToCache. The patch thus renames MaybeLoadBlocksToCache to RetrieveMultipleBlock and deletes GetDataBlockFromCache, which contains only two lines.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5726

Differential Revision: D16962535

Pulled By: maysamyabandeh

fbshipit-source-id: 99e8946808ce4eb7857592b9003812e3004f92d6
---
 table/block_based/block_based_table_reader.cc | 39 +++++++------------
 table/block_based/block_based_table_reader.h  |  8 +---
 2 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc
index bf0bed846..b3bca0cb8 100644
--- a/table/block_based/block_based_table_reader.cc
+++ b/table/block_based/block_based_table_reader.cc
@@ -2080,27 +2080,6 @@ TBlockIter* BlockBasedTable::NewDataBlockIterator(
   return iter;
 }
 
-// Lookup the cache for the given data block referenced by an index iterator
-// value (i.e BlockHandle). If it exists in the cache, initialize block to
-// the contents of the data block.
-Status BlockBasedTable::GetDataBlockFromCache(
-    const ReadOptions& ro, const BlockHandle& handle,
-    const UncompressionDict& uncompression_dict,
-    CachableEntry<Block>* block, BlockType block_type,
-    GetContext* get_context) const {
-  BlockCacheLookupContext lookup_data_block_context(
-      TableReaderCaller::kUserMultiGet);
-  assert(block_type == BlockType::kData);
-  Status s = RetrieveBlock(nullptr, ro, handle, uncompression_dict, block,
-                           block_type, get_context, &lookup_data_block_context,
-                           /* for_compaction */ false, /* use_cache */ true);
-  if (s.IsIncomplete()) {
-    s = Status::OK();
-  }
-
-  return s;
-}
-
 // If contents is nullptr, this function looks up the block caches for the
 // data block referenced by handle, and read the block from disk if necessary.
 // If contents is non-null, it skips the cache lookup and disk read, since
@@ -2275,7 +2254,7 @@ Status BlockBasedTable::MaybeReadBlockAndLoadToCache(
 //         found in cache
 // handles - A vector of block handles. Some of them me be NULL handles
 // scratch - An optional contiguous buffer to read compressed blocks into
-void BlockBasedTable::MaybeLoadBlocksToCache(
+void BlockBasedTable::RetrieveMultipleBlocks(
     const ReadOptions& options,
     const MultiGetRange* batch,
     const autovector<BlockHandle, MultiGetContext::MAX_BATCH_SIZE>*  handles,
@@ -3441,10 +3420,20 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
           block_handles.emplace_back(BlockHandle::NullBlockHandle());
           continue;
         }
+        // Lookup the cache for the given data block referenced by an index
+        // iterator value (i.e BlockHandle). If it exists in the cache,
+        // initialize block to the contents of the data block.
         offset = v.handle.offset();
         BlockHandle handle = v.handle;
-        Status s = GetDataBlockFromCache(ro, handle, uncompression_dict,
-              &(results.back()), BlockType::kData, miter->get_context);
+        BlockCacheLookupContext lookup_data_block_context(
+            TableReaderCaller::kUserMultiGet);
+        Status s = RetrieveBlock(nullptr, ro, handle, uncompression_dict,
+                          &(results.back()), BlockType::kData,
+                          miter->get_context, &lookup_data_block_context,
+                          /* for_compaction */ false, /* use_cache */ true);
+        if (s.IsIncomplete()) {
+          s = Status::OK();
+        }
         if (s.ok() && !results.back().IsEmpty()) {
           // Found it in the cache. Add NULL handle to indicate there is
           // nothing to read from disk
@@ -3475,7 +3464,7 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,
             block_buf.reset(scratch);
           }
         }
-        MaybeLoadBlocksToCache(read_options,
+        RetrieveMultipleBlocks(read_options,
             &data_block_range, &block_handles, &statuses, &results,
             scratch, uncompression_dict);
       }
diff --git a/table/block_based/block_based_table_reader.h b/table/block_based/block_based_table_reader.h
index 7597c00d6..3609db26e 100644
--- a/table/block_based/block_based_table_reader.h
+++ b/table/block_based/block_based_table_reader.h
@@ -316,13 +316,7 @@ class BlockBasedTable : public TableReader {
                        BlockCacheLookupContext* lookup_context,
                        bool for_compaction, bool use_cache) const;
 
-  Status GetDataBlockFromCache(
-      const ReadOptions& ro, const BlockHandle& handle,
-      const UncompressionDict& uncompression_dict,
-      CachableEntry<Block>* block_entry, BlockType block_type,
-      GetContext* get_context) const;
-
-  void MaybeLoadBlocksToCache(
+  void RetrieveMultipleBlocks(
       const ReadOptions& options, const MultiGetRange* batch,
       const autovector<BlockHandle, MultiGetContext::MAX_BATCH_SIZE>*  handles,
       autovector<Status, MultiGetContext::MAX_BATCH_SIZE>* statuses,