Skip to content

Commit

Permalink
Propagate fill_cache config to partitioned index iterator
Browse files Browse the repository at this point in the history
Summary:
Currently the partitioned index iterator creates a new ReadOptions which ignores the fill_cache config set to ReadOptions passed by the user. The patch propagates fill_cache from the user's ReadOptions to that of partition index iterator.
Also it clarifies the contract of fill_cache that i) it does not apply to filters, ii) it still charges block cache for the size of the data block, it still pin the block if it is already in the block cache.
Closes #3739

Differential Revision: D7678308

Pulled By: maysamyabandeh

fbshipit-source-id: 53ed96424ae922e499e2d4e3580ddc3f0db893da
  • Loading branch information
Maysam Yabandeh authored and facebook-github-bot committed Apr 20, 2018
1 parent dee95a1 commit 17e0403
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
7 changes: 4 additions & 3 deletions include/rocksdb/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -1033,10 +1033,11 @@ struct ReadOptions {
// Default: true
bool verify_checksums;

// Should the "data block"/"index block"/"filter block" read for this
// iteration be cached in memory?
// Should the "data block"/"index block"" read for this iteration be placed in
// block cache?
// Callers may wish to set this field to false for bulk scans.
// Default: true
// This would help not to the change eviction order of existing items in the
// block cache. Default: true
bool fill_cache;

// Specify to create a tailing iterator -- a special iterator that has a
Expand Down
17 changes: 11 additions & 6 deletions table/block_based_table_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,19 @@ class PartitionIndexReader : public IndexReader, public Cleanable {

// return a two-level iterator: first level is on the partition index
virtual InternalIterator* NewIterator(BlockIter* /*iter*/ = nullptr,
bool /*dont_care*/ = true) override {
bool /*dont_care*/ = true,
bool fill_cache = true) override {
// Filters are already checked before seeking the index
if (!partition_map_.empty()) {
return NewTwoLevelIterator(
new BlockBasedTable::PartitionedIndexIteratorState(
table_, partition_map_.size() ? &partition_map_ : nullptr),
index_block_->NewIterator(icomparator_, nullptr, true));
} else {
auto ro = ReadOptions();
ro.fill_cache = fill_cache;
return new BlockBasedTableIterator(
table_, ReadOptions(), *icomparator_,
table_, ro, *icomparator_,
index_block_->NewIterator(icomparator_, nullptr, true), false);
}
// TODO(myabandeh): Update TwoLevelIterator to be able to make use of
Expand Down Expand Up @@ -364,6 +367,7 @@ class BinarySearchIndexReader : public IndexReader {
}

virtual InternalIterator* NewIterator(BlockIter* iter = nullptr,
bool /*dont_care*/ = true,
bool /*dont_care*/ = true) override {
return index_block_->NewIterator(icomparator_, iter, true);
}
Expand Down Expand Up @@ -474,7 +478,8 @@ class HashIndexReader : public IndexReader {
}

virtual InternalIterator* NewIterator(BlockIter* iter = nullptr,
bool total_order_seek = true) override {
bool total_order_seek = true,
bool /*dont_care*/ = true) override {
return index_block_->NewIterator(icomparator_, iter, total_order_seek);
}

Expand Down Expand Up @@ -1364,12 +1369,12 @@ InternalIterator* BlockBasedTable::NewIndexIterator(
// index reader has already been pre-populated.
if (rep_->index_reader) {
return rep_->index_reader->NewIterator(
input_iter, read_options.total_order_seek);
input_iter, read_options.total_order_seek, read_options.fill_cache);
}
// we have a pinned index block
if (rep_->index_entry.IsSet()) {
return rep_->index_entry.value->NewIterator(input_iter,
read_options.total_order_seek);
return rep_->index_entry.value->NewIterator(
input_iter, read_options.total_order_seek, read_options.fill_cache);
}

PERF_TIMER_GUARD(read_index_block_nanos);
Expand Down
3 changes: 2 additions & 1 deletion table/block_based_table_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ class BlockBasedTable : public TableReader {
// a different object then iter and the callee has the ownership of the
// returned object.
virtual InternalIterator* NewIterator(BlockIter* iter = nullptr,
bool total_order_seek = true) = 0;
bool total_order_seek = true,
bool fill_cache = true) = 0;

// The size of the index.
virtual size_t size() const = 0;
Expand Down

0 comments on commit 17e0403

Please sign in to comment.