Revert to respecting only the read_tier read option for index blocks (#5481)

Summary:
PR https://github.com/facebook/rocksdb/issues/5298 subtly changed how read options are applied to the index block
during a Get, MultiGet, or iteration. Earlier, only the read_tier option
applied to the index block read; since PR https://github.com/facebook/rocksdb/issues/5298, fill_cache and
verify_checksums also have an effect. This patch restores the earlier
behavior to prevent surprise memory increases for clients due to the
index block not being cached.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5481

Test Plan: make check

Differential Revision: D15883082

Pulled By: ltamasi

fbshipit-source-id: 9a065ec3a6db5a365cf6dd5e95190a20c5756356
main
Levi Tamasi 6 years ago committed by Facebook Github Bot
parent 220870523c
commit d0c6aea192
  1. 27
      table/block_based/block_based_table_reader.cc

@ -210,8 +210,7 @@ class BlockBasedTable::IndexReaderCommon : public BlockBasedTable::IndexReader {
return properties == nullptr || !properties->index_value_is_delta_encoded; return properties == nullptr || !properties->index_value_is_delta_encoded;
} }
Status GetOrReadIndexBlock(const ReadOptions& read_options, Status GetOrReadIndexBlock(bool no_io, GetContext* get_context,
GetContext* get_context,
BlockCacheLookupContext* lookup_context, BlockCacheLookupContext* lookup_context,
CachableEntry<Block>* index_block) const; CachableEntry<Block>* index_block) const;
@ -250,7 +249,7 @@ Status BlockBasedTable::IndexReaderCommon::ReadIndexBlock(
} }
Status BlockBasedTable::IndexReaderCommon::GetOrReadIndexBlock( Status BlockBasedTable::IndexReaderCommon::GetOrReadIndexBlock(
const ReadOptions& read_options, GetContext* get_context, bool no_io, GetContext* get_context,
BlockCacheLookupContext* lookup_context, BlockCacheLookupContext* lookup_context,
CachableEntry<Block>* index_block) const { CachableEntry<Block>* index_block) const {
assert(index_block != nullptr); assert(index_block != nullptr);
@ -260,6 +259,11 @@ Status BlockBasedTable::IndexReaderCommon::GetOrReadIndexBlock(
return Status::OK(); return Status::OK();
} }
ReadOptions read_options;
if (no_io) {
read_options.read_tier = kBlockCacheTier;
}
return ReadIndexBlock(table_, /*prefetch_buffer=*/nullptr, read_options, return ReadIndexBlock(table_, /*prefetch_buffer=*/nullptr, read_options,
get_context, lookup_context, index_block); get_context, lookup_context, index_block);
} }
@ -304,9 +308,10 @@ class PartitionIndexReader : public BlockBasedTable::IndexReaderCommon {
const ReadOptions& read_options, bool /* disable_prefix_seek */, const ReadOptions& read_options, bool /* disable_prefix_seek */,
IndexBlockIter* iter, GetContext* get_context, IndexBlockIter* iter, GetContext* get_context,
BlockCacheLookupContext* lookup_context) override { BlockCacheLookupContext* lookup_context) override {
const bool no_io = (read_options.read_tier == kBlockCacheTier);
CachableEntry<Block> index_block; CachableEntry<Block> index_block;
const Status s = GetOrReadIndexBlock(read_options, get_context, const Status s =
lookup_context, &index_block); GetOrReadIndexBlock(no_io, get_context, lookup_context, &index_block);
if (!s.ok()) { if (!s.ok()) {
if (iter != nullptr) { if (iter != nullptr) {
iter->Invalidate(s); iter->Invalidate(s);
@ -366,7 +371,7 @@ class PartitionIndexReader : public BlockBasedTable::IndexReaderCommon {
Statistics* kNullStats = nullptr; Statistics* kNullStats = nullptr;
CachableEntry<Block> index_block; CachableEntry<Block> index_block;
Status s = GetOrReadIndexBlock(ReadOptions(), nullptr /* get_context */, Status s = GetOrReadIndexBlock(false /* no_io */, nullptr /* get_context */,
&lookup_context, &index_block); &lookup_context, &index_block);
if (!s.ok()) { if (!s.ok()) {
ROCKS_LOG_WARN(rep->ioptions.info_log, ROCKS_LOG_WARN(rep->ioptions.info_log,
@ -489,9 +494,10 @@ class BinarySearchIndexReader : public BlockBasedTable::IndexReaderCommon {
const ReadOptions& read_options, bool /* disable_prefix_seek */, const ReadOptions& read_options, bool /* disable_prefix_seek */,
IndexBlockIter* iter, GetContext* get_context, IndexBlockIter* iter, GetContext* get_context,
BlockCacheLookupContext* lookup_context) override { BlockCacheLookupContext* lookup_context) override {
const bool no_io = (read_options.read_tier == kBlockCacheTier);
CachableEntry<Block> index_block; CachableEntry<Block> index_block;
const Status s = GetOrReadIndexBlock(read_options, get_context, const Status s =
lookup_context, &index_block); GetOrReadIndexBlock(no_io, get_context, lookup_context, &index_block);
if (!s.ok()) { if (!s.ok()) {
if (iter != nullptr) { if (iter != nullptr) {
iter->Invalidate(s); iter->Invalidate(s);
@ -631,9 +637,10 @@ class HashIndexReader : public BlockBasedTable::IndexReaderCommon {
const ReadOptions& read_options, bool disable_prefix_seek, const ReadOptions& read_options, bool disable_prefix_seek,
IndexBlockIter* iter, GetContext* get_context, IndexBlockIter* iter, GetContext* get_context,
BlockCacheLookupContext* lookup_context) override { BlockCacheLookupContext* lookup_context) override {
const bool no_io = (read_options.read_tier == kBlockCacheTier);
CachableEntry<Block> index_block; CachableEntry<Block> index_block;
const Status s = GetOrReadIndexBlock(read_options, get_context, const Status s =
lookup_context, &index_block); GetOrReadIndexBlock(no_io, get_context, lookup_context, &index_block);
if (!s.ok()) { if (!s.ok()) {
if (iter != nullptr) { if (iter != nullptr) {
iter->Invalidate(s); iter->Invalidate(s);

Loading…
Cancel
Save