From e70020e4f64b7ce190006ab812a1371dbd690eea Mon Sep 17 00:00:00 2001 From: omegaga Date: Wed, 20 Jul 2016 11:23:31 -0700 Subject: [PATCH] Only cache level 0 indexes and filter when opening table reader Summary: In T8216281 we decided to disable prefetching the index and filter during opening table handlers during startup (max_open_files = -1). Test Plan: Rely on `IndexAndFilterBlocksOfNewTableAddedToCache` to guarantee L0 indexes and filters are still cached and change `PinL0IndexAndFilterBlocksTest` to make sure other levels are not cached (maybe add one more test to test we don't cache other levels?) Reviewers: sdong, andrewkr Reviewed By: andrewkr Subscribers: andrewkr, dhruba Differential Revision: https://reviews.facebook.net/D59913 --- db/db_test2.cc | 126 +++++++++++++++++++------ db/plain_table_db_test.cc | 9 +- db/table_cache.cc | 10 +- db/table_cache.h | 6 +- db/version_builder.cc | 22 +++-- db/version_builder.h | 3 +- db/version_set.cc | 8 +- include/rocksdb/table.h | 3 +- table/adaptive_table_factory.cc | 3 +- table/adaptive_table_factory.h | 9 +- table/block_based_table_factory.cc | 14 +-- table/block_based_table_factory.h | 17 +--- table/block_based_table_reader.cc | 39 ++++---- table/block_based_table_reader.h | 10 +- table/cuckoo_table_factory.cc | 3 +- table/cuckoo_table_factory.h | 9 +- table/mock_table.cc | 3 +- table/mock_table.h | 9 +- table/plain_table_factory.cc | 3 +- table/plain_table_factory.h | 4 +- utilities/options/options_util_test.cc | 3 +- 21 files changed, 195 insertions(+), 118 deletions(-) diff --git a/db/db_test2.cc b/db/db_test2.cc index 4fe886fbe..147715a7d 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -1210,6 +1210,37 @@ class PinL0IndexAndFilterBlocksTest : public DBTestBase, PinL0IndexAndFilterBlocksTest() : DBTestBase("/db_pin_l0_index_bloom_test") {} virtual void SetUp() override { infinite_max_files_ = GetParam(); } + void CreateTwoLevels(Options* options) { + if (infinite_max_files_) { + options->max_open_files = -1; + } + options->create_if_missing = true; + options->statistics = rocksdb::CreateDBStatistics(); + BlockBasedTableOptions table_options; + table_options.cache_index_and_filter_blocks = true; + table_options.pin_l0_filter_and_index_blocks_in_cache = true; + table_options.filter_policy.reset(NewBloomFilterPolicy(20)); + options->table_factory.reset(new BlockBasedTableFactory(table_options)); + CreateAndReopenWithCF({"pikachu"}, *options); + + Put(1, "a", "begin"); + Put(1, "z", "end"); + ASSERT_OK(Flush(1)); + // move this table to L1 + dbfull()->TEST_CompactRange(0, nullptr, nullptr, handles_[1]); + + // reset block cache + table_options.block_cache = NewLRUCache(64 * 1024); + options->table_factory.reset(NewBlockBasedTableFactory(table_options)); + TryReopenWithColumnFamilies({"default", "pikachu"}, *options); + // create new table at L0 + Put(1, "a2", "begin2"); + Put(1, "z2", "end2"); + ASSERT_OK(Flush(1)); + + table_options.block_cache->EraseUnRefEntries(); + } + bool infinite_max_files_; }; @@ -1261,35 +1292,7 @@ TEST_P(PinL0IndexAndFilterBlocksTest, TEST_P(PinL0IndexAndFilterBlocksTest, MultiLevelIndexAndFilterBlocksCachedWithPinning) { Options options = CurrentOptions(); - if (infinite_max_files_) { - options.max_open_files = -1; - } - options.create_if_missing = true; - options.statistics = rocksdb::CreateDBStatistics(); - BlockBasedTableOptions table_options; - table_options.cache_index_and_filter_blocks = true; - table_options.pin_l0_filter_and_index_blocks_in_cache = true; - table_options.filter_policy.reset(NewBloomFilterPolicy(20)); - options.table_factory.reset(new BlockBasedTableFactory(table_options)); - CreateAndReopenWithCF({"pikachu"}, options); - - Put(1, "a", "begin"); - Put(1, "z", "end"); - ASSERT_OK(Flush(1)); - // move this table to L1 - dbfull()->TEST_CompactRange(0, nullptr, nullptr, handles_[1]); - - // reset block cache - table_options.block_cache = NewLRUCache(64 * 1024); - options.table_factory.reset(NewBlockBasedTableFactory(table_options)); - TryReopenWithColumnFamilies({"default", "pikachu"}, options); - // create new table at L0 - Put(1, "a2", "begin2"); - Put(1, "z2", "end2"); - ASSERT_OK(Flush(1)); - - table_options.block_cache->EraseUnRefEntries(); - + PinL0IndexAndFilterBlocksTest::CreateTwoLevels(&options); // get base cache values uint64_t fm = TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS); uint64_t fh = TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT); @@ -1309,11 +1312,76 @@ TEST_P(PinL0IndexAndFilterBlocksTest, // the file is opened, prefetching results in a cache filter miss // the block is loaded and added to the cache, // then the get results in a cache hit for L1 + // When we have inifinite max_files, there is still cache miss because we have + // reset the block cache value = Get(1, "a"); ASSERT_EQ(fm + 1, TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS)); ASSERT_EQ(im + 1, TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS)); } +TEST_P(PinL0IndexAndFilterBlocksTest, DisablePrefetchingNonL0IndexAndFilter) { + Options options = CurrentOptions(); + PinL0IndexAndFilterBlocksTest::CreateTwoLevels(&options); + + // Get base cache values + uint64_t fm = TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS); + uint64_t fh = TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT); + uint64_t im = TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS); + uint64_t ih = TestGetTickerCount(options, BLOCK_CACHE_INDEX_HIT); + + // Reopen database. If max_open_files is set as -1, table readers will be + // preloaded. This will trigger a BlockBasedTable::Open() and prefetch + // L0 index and filter. Level 1's prefetching is disabled in DB::Open() + TryReopenWithColumnFamilies({"default", "pikachu"}, options); + + if (infinite_max_files_) { + // After reopen, cache miss are increased by one because we read (and only + // read) filter and index on L0 + ASSERT_EQ(fm + 1, TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS)); + ASSERT_EQ(fh, TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT)); + ASSERT_EQ(im + 1, TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS)); + ASSERT_EQ(ih, TestGetTickerCount(options, BLOCK_CACHE_INDEX_HIT)); + } else { + // If max_open_files is not -1, we do not preload table readers, so there is + // no change. + ASSERT_EQ(fm, TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS)); + ASSERT_EQ(fh, TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT)); + ASSERT_EQ(im, TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS)); + ASSERT_EQ(ih, TestGetTickerCount(options, BLOCK_CACHE_INDEX_HIT)); + } + std::string value; + // this should be read from L0 + value = Get(1, "a2"); + // If max_open_files is -1, we have pinned index and filter in Rep, so there + // will not be changes in index and filter misses or hits. If max_open_files + // is not -1, Get() will open a TableReader and prefetch index and filter. + ASSERT_EQ(fm + 1, TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS)); + ASSERT_EQ(fh, TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT)); + ASSERT_EQ(im + 1, TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS)); + ASSERT_EQ(ih, TestGetTickerCount(options, BLOCK_CACHE_INDEX_HIT)); + + // this should be read from L1 + value = Get(1, "a"); + if (infinite_max_files_) { + // In inifinite max files case, there's a cache miss in executing Get() + // because index and filter are not prefetched before. + ASSERT_EQ(fm + 2, TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS)); + ASSERT_EQ(fh, TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT)); + ASSERT_EQ(im + 2, TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS)); + ASSERT_EQ(ih, TestGetTickerCount(options, BLOCK_CACHE_INDEX_HIT)); + } else { + // In this case, cache miss will be increased by one in + // BlockBasedTable::Open() because this is not in DB::Open() code path so we + // will prefetch L1's index and filter. Cache hit will also be increased by + // one because Get() will read index and filter from the block cache + // prefetched in previous Open() call. + ASSERT_EQ(fm + 2, TestGetTickerCount(options, BLOCK_CACHE_FILTER_MISS)); + ASSERT_EQ(fh + 1, TestGetTickerCount(options, BLOCK_CACHE_FILTER_HIT)); + ASSERT_EQ(im + 2, TestGetTickerCount(options, BLOCK_CACHE_INDEX_MISS)); + ASSERT_EQ(ih + 1, TestGetTickerCount(options, BLOCK_CACHE_INDEX_HIT)); + } +} + INSTANTIATE_TEST_CASE_P(PinL0IndexAndFilterBlocksTest, PinL0IndexAndFilterBlocksTest, ::testing::Bool()); #ifndef ROCKSDB_LITE diff --git a/db/plain_table_db_test.cc b/db/plain_table_db_test.cc index 9d3803f9e..fdf98b0bf 100644 --- a/db/plain_table_db_test.cc +++ b/db/plain_table_db_test.cc @@ -324,10 +324,11 @@ class TestPlainTableFactory : public PlainTableFactory { column_family_id_(column_family_id), column_family_name_(std::move(column_family_name)) {} - Status NewTableReader(const TableReaderOptions& table_reader_options, - unique_ptr&& file, - uint64_t file_size, - unique_ptr* table) const override { + Status NewTableReader( + const TableReaderOptions& table_reader_options, + unique_ptr&& file, uint64_t file_size, + unique_ptr* table, + bool prefetch_index_and_filter_in_cache) const override { TableProperties* props = nullptr; auto s = ReadTableProperties(file.get(), file_size, kPlainTableMagicNumber, diff --git a/db/table_cache.cc b/db/table_cache.cc index 9b0e8edb9..5352309f4 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -89,7 +89,7 @@ Status TableCache::GetTableReader( const InternalKeyComparator& internal_comparator, const FileDescriptor& fd, bool sequential_mode, size_t readahead, bool record_read_stats, HistogramImpl* file_read_hist, unique_ptr* table_reader, - bool skip_filters, int level) { + bool skip_filters, int level, bool prefetch_index_and_filter_in_cache) { std::string fname = TableFileName(ioptions_.db_paths, fd.GetNumber(), fd.GetPathId()); unique_ptr file; @@ -111,7 +111,8 @@ Status TableCache::GetTableReader( s = ioptions_.table_factory->NewTableReader( TableReaderOptions(ioptions_, env_options, internal_comparator, skip_filters, level), - std::move(file_reader), fd.GetFileSize(), table_reader); + std::move(file_reader), fd.GetFileSize(), table_reader, + prefetch_index_and_filter_in_cache); TEST_SYNC_POINT("TableCache::GetTableReader:0"); } return s; @@ -129,7 +130,8 @@ Status TableCache::FindTable(const EnvOptions& env_options, const FileDescriptor& fd, Cache::Handle** handle, const bool no_io, bool record_read_stats, HistogramImpl* file_read_hist, bool skip_filters, - int level) { + int level, + bool prefetch_index_and_filter_in_cache) { PERF_TIMER_GUARD(find_table_nanos); Status s; uint64_t number = fd.GetNumber(); @@ -146,7 +148,7 @@ Status TableCache::FindTable(const EnvOptions& env_options, s = GetTableReader(env_options, internal_comparator, fd, false /* sequential mode */, 0 /* readahead */, record_read_stats, file_read_hist, &table_reader, - skip_filters, level); + skip_filters, level, prefetch_index_and_filter_in_cache); if (!s.ok()) { assert(table_reader == nullptr); RecordTick(ioptions_.statistics, NO_FILE_ERRORS); diff --git a/db/table_cache.h b/db/table_cache.h index 18882c6a2..cdd0f23dc 100644 --- a/db/table_cache.h +++ b/db/table_cache.h @@ -79,7 +79,8 @@ class TableCache { const FileDescriptor& file_fd, Cache::Handle**, const bool no_io = false, bool record_read_stats = true, HistogramImpl* file_read_hist = nullptr, - bool skip_filters = false, int level = -1); + bool skip_filters = false, int level = -1, + bool prefetch_index_and_filter_in_cache = true); // Get TableReader from a cache handle. TableReader* GetTableReaderFromHandle(Cache::Handle* handle); @@ -114,7 +115,8 @@ class TableCache { size_t readahead, bool record_read_stats, HistogramImpl* file_read_hist, unique_ptr* table_reader, - bool skip_filters = false, int level = -1); + bool skip_filters = false, int level = -1, + bool prefetch_index_and_filter_in_cache = true); const ImmutableCFOptions& ioptions_; const EnvOptions& env_options_; diff --git a/db/version_builder.cc b/db/version_builder.cc index d73b33d8e..2837686be 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -287,7 +287,8 @@ class VersionBuilder::Rep { CheckConsistency(vstorage); } - void LoadTableHandlers(InternalStats* internal_stats, int max_threads) { + void LoadTableHandlers(InternalStats* internal_stats, int max_threads, + bool prefetch_index_and_filter_in_cache) { assert(table_cache_ != nullptr); // std::vector> files_meta; @@ -309,11 +310,12 @@ class VersionBuilder::Rep { auto* file_meta = files_meta[file_idx].first; int level = files_meta[file_idx].second; - table_cache_->FindTable( - env_options_, *(base_vstorage_->InternalComparator()), - file_meta->fd, &file_meta->table_reader_handle, false /*no_io */, - true /* record_read_stats */, - internal_stats->GetFileReadHist(level), false, level); + table_cache_->FindTable(env_options_, + *(base_vstorage_->InternalComparator()), + file_meta->fd, &file_meta->table_reader_handle, + false /*no_io */, true /* record_read_stats */, + internal_stats->GetFileReadHist(level), false, + level, prefetch_index_and_filter_in_cache); if (file_meta->table_reader_handle != nullptr) { // Load table_reader file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( @@ -363,9 +365,11 @@ void VersionBuilder::Apply(VersionEdit* edit) { rep_->Apply(edit); } void VersionBuilder::SaveTo(VersionStorageInfo* vstorage) { rep_->SaveTo(vstorage); } -void VersionBuilder::LoadTableHandlers(InternalStats* internal_stats, - int max_threads) { - rep_->LoadTableHandlers(internal_stats, max_threads); +void VersionBuilder::LoadTableHandlers( + InternalStats* internal_stats, int max_threads, + bool prefetch_index_and_filter_in_cache) { + rep_->LoadTableHandlers(internal_stats, max_threads, + prefetch_index_and_filter_in_cache); } void VersionBuilder::MaybeAddFile(VersionStorageInfo* vstorage, int level, FileMetaData* f) { diff --git a/db/version_builder.h b/db/version_builder.h index c09815217..44ff75939 100644 --- a/db/version_builder.h +++ b/db/version_builder.h @@ -31,7 +31,8 @@ class VersionBuilder { int level); void Apply(VersionEdit* edit); void SaveTo(VersionStorageInfo* vstorage); - void LoadTableHandlers(InternalStats* internal_stats, int max_threads = 1); + void LoadTableHandlers(InternalStats* internal_stats, int max_threads, + bool prefetch_index_and_filter_in_cache); void MaybeAddFile(VersionStorageInfo* vstorage, int level, FileMetaData* f); private: diff --git a/db/version_set.cc b/db/version_set.cc index a048d6d9b..8bd6890ac 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -2272,7 +2272,8 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data, // Need to do it out of the mutex. builder_guard->version_builder()->LoadTableHandlers( column_family_data->internal_stats(), - column_family_data->ioptions()->optimize_filters_for_hits); + column_family_data->ioptions()->optimize_filters_for_hits, + true /* prefetch_index_and_filter_in_cache */); } // This is fine because everything inside of this block is serialized -- @@ -2714,8 +2715,9 @@ Status VersionSet::Recover( if (db_options_->max_open_files == -1) { // unlimited table cache. Pre-load table handle now. // Need to do it out of the mutex. - builder->LoadTableHandlers(cfd->internal_stats(), - db_options_->max_file_opening_threads); + builder->LoadTableHandlers( + cfd->internal_stats(), db_options_->max_file_opening_threads, + false /* prefetch_index_and_filter_in_cache */); } Version* v = new Version(cfd, this, current_version_number_++); diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h index f7a219504..333275316 100644 --- a/include/rocksdb/table.h +++ b/include/rocksdb/table.h @@ -390,7 +390,8 @@ class TableFactory { virtual Status NewTableReader( const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - unique_ptr* table_reader) const = 0; + unique_ptr* table_reader, + bool prefetch_index_and_filter_in_cache = true) const = 0; // Return a table builder to write to a file for this table type. // diff --git a/table/adaptive_table_factory.cc b/table/adaptive_table_factory.cc index c7d40b9c8..bb19c417e 100644 --- a/table/adaptive_table_factory.cc +++ b/table/adaptive_table_factory.cc @@ -43,7 +43,8 @@ extern const uint64_t kCuckooTableMagicNumber; Status AdaptiveTableFactory::NewTableReader( const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - unique_ptr* table) const { + unique_ptr* table, + bool prefetch_index_and_filter_in_cache) const { Footer footer; auto s = ReadFooterFromFile(file.get(), file_size, &footer); if (!s.ok()) { diff --git a/table/adaptive_table_factory.h b/table/adaptive_table_factory.h index 955ded4d2..b7b52ba96 100644 --- a/table/adaptive_table_factory.h +++ b/table/adaptive_table_factory.h @@ -33,10 +33,11 @@ class AdaptiveTableFactory : public TableFactory { const char* Name() const override { return "AdaptiveTableFactory"; } - Status NewTableReader(const TableReaderOptions& table_reader_options, - unique_ptr&& file, - uint64_t file_size, - unique_ptr* table) const override; + Status NewTableReader( + const TableReaderOptions& table_reader_options, + unique_ptr&& file, uint64_t file_size, + unique_ptr* table, + bool prefetch_index_and_filter_in_cache = true) const override; TableBuilder* NewTableBuilder( const TableBuilderOptions& table_builder_options, diff --git a/table/block_based_table_factory.cc b/table/block_based_table_factory.cc index 333da56ab..e9499cd67 100644 --- a/table/block_based_table_factory.cc +++ b/table/block_based_table_factory.cc @@ -50,20 +50,12 @@ BlockBasedTableFactory::BlockBasedTableFactory( Status BlockBasedTableFactory::NewTableReader( const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - unique_ptr* table_reader) const { - return NewTableReader(table_reader_options, std::move(file), file_size, - table_reader, - /*prefetch_index_and_filter=*/true); -} - -Status BlockBasedTableFactory::NewTableReader( - const TableReaderOptions& table_reader_options, - unique_ptr&& file, uint64_t file_size, - unique_ptr* table_reader, const bool prefetch_enabled) const { + unique_ptr* table_reader, + bool prefetch_index_and_filter_in_cache) const { return BlockBasedTable::Open( table_reader_options.ioptions, table_reader_options.env_options, table_options_, table_reader_options.internal_comparator, std::move(file), - file_size, table_reader, prefetch_enabled, + file_size, table_reader, prefetch_index_and_filter_in_cache, table_reader_options.skip_filters, table_reader_options.level); } diff --git a/table/block_based_table_factory.h b/table/block_based_table_factory.h index 6b4e563e2..6ecb232e4 100644 --- a/table/block_based_table_factory.h +++ b/table/block_based_table_factory.h @@ -33,18 +33,11 @@ class BlockBasedTableFactory : public TableFactory { const char* Name() const override { return "BlockBasedTable"; } - Status NewTableReader(const TableReaderOptions& table_reader_options, - unique_ptr&& file, - uint64_t file_size, - unique_ptr* table_reader) const override; - - // This is a variant of virtual member function NewTableReader function with - // added capability to disable pre-fetching of blocks on BlockBasedTable::Open - Status NewTableReader(const TableReaderOptions& table_reader_options, - unique_ptr&& file, - uint64_t file_size, - unique_ptr* table_reader, - bool prefetch_index_and_filter) const; + Status NewTableReader( + const TableReaderOptions& table_reader_options, + unique_ptr&& file, uint64_t file_size, + unique_ptr* table_reader, + bool prefetch_index_and_filter_in_cache = true) const override; TableBuilder* NewTableBuilder( const TableBuilderOptions& table_builder_options, diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index 4de61817b..bcb803217 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -515,7 +515,7 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, unique_ptr&& file, uint64_t file_size, unique_ptr* table_reader, - const bool prefetch_index_and_filter, + const bool prefetch_index_and_filter_in_cache, const bool skip_filters, const int level) { table_reader->reset(); @@ -636,10 +636,11 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, BlockBasedTablePropertyNames::kPrefixFiltering, rep->ioptions.info_log); } - if (prefetch_index_and_filter) { // pre-fetching of blocks is turned on - // Will use block cache for index/filter blocks access? - if (table_options.cache_index_and_filter_blocks) { + // Will use block cache for index/filter blocks access + // Always prefetch index and filter for level 0 + if (table_options.cache_index_and_filter_blocks) { + if (prefetch_index_and_filter_in_cache || level == 0) { assert(table_options.block_cache != nullptr); // Hack: Call NewIndexIterator() to implicitly add index to the // block_cache @@ -663,7 +664,7 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, // if pin_l0_filter_and_index_blocks_in_cache is true, and this is // a level0 file, then save it in rep_->filter_entry; it will be // released in the destructor only, hence it will be pinned in the - // cache until this reader is alive + // cache while this reader is alive if (rep->table_options.pin_l0_filter_and_index_blocks_in_cache && level == 0) { rep->filter_entry = filter_entry; @@ -671,23 +672,23 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, filter_entry.Release(table_options.block_cache.get()); } } - } else { - // If we don't use block cache for index/filter blocks access, we'll - // pre-load these blocks, which will kept in member variables in Rep - // and with a same life-time as this table object. - IndexReader* index_reader = nullptr; - s = new_table->CreateIndexReader(&index_reader, meta_iter.get()); + } + } else { + // If we don't use block cache for index/filter blocks access, we'll + // pre-load these blocks, which will kept in member variables in Rep + // and with a same life-time as this table object. + IndexReader* index_reader = nullptr; + s = new_table->CreateIndexReader(&index_reader, meta_iter.get()); - if (s.ok()) { - rep->index_reader.reset(index_reader); + if (s.ok()) { + rep->index_reader.reset(index_reader); - // Set filter block - if (rep->filter_policy) { - rep->filter.reset(ReadFilter(rep)); - } - } else { - delete index_reader; + // Set filter block + if (rep->filter_policy) { + rep->filter.reset(ReadFilter(rep)); } + } else { + delete index_reader; } } diff --git a/table/block_based_table_reader.h b/table/block_based_table_reader.h index 84a990a6f..9c8b1b114 100644 --- a/table/block_based_table_reader.h +++ b/table/block_based_table_reader.h @@ -69,17 +69,19 @@ class BlockBasedTable : public TableReader { // to nullptr and returns a non-ok status. // // @param file must remain live while this Table is in use. - // @param prefetch_index_and_filter can be used to disable prefetching of - // index and filter blocks at startup + // @param prefetch_index_and_filter_in_cache can be used to disable + // prefetching of + // index and filter blocks into block cache at startup // @param skip_filters Disables loading/accessing the filter block. Overrides - // prefetch_index_and_filter, so filter will be skipped if both are set. + // prefetch_index_and_filter_in_cache, so filter will be skipped if both + // are set. static Status Open(const ImmutableCFOptions& ioptions, const EnvOptions& env_options, const BlockBasedTableOptions& table_options, const InternalKeyComparator& internal_key_comparator, unique_ptr&& file, uint64_t file_size, unique_ptr* table_reader, - bool prefetch_index_and_filter = true, + bool prefetch_index_and_filter_in_cache = true, bool skip_filters = false, int level = -1); bool PrefixMayMatch(const Slice& internal_key); diff --git a/table/cuckoo_table_factory.cc b/table/cuckoo_table_factory.cc index 36a938571..87107ee7f 100644 --- a/table/cuckoo_table_factory.cc +++ b/table/cuckoo_table_factory.cc @@ -15,7 +15,8 @@ namespace rocksdb { Status CuckooTableFactory::NewTableReader( const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - std::unique_ptr* table) const { + std::unique_ptr* table, + bool prefetch_index_and_filter_in_cache) const { std::unique_ptr new_reader(new CuckooTableReader( table_reader_options.ioptions, std::move(file), file_size, table_reader_options.internal_comparator.user_comparator(), nullptr)); diff --git a/table/cuckoo_table_factory.h b/table/cuckoo_table_factory.h index 82aa57150..ff67ae247 100644 --- a/table/cuckoo_table_factory.h +++ b/table/cuckoo_table_factory.h @@ -55,10 +55,11 @@ class CuckooTableFactory : public TableFactory { const char* Name() const override { return "CuckooTable"; } - Status NewTableReader(const TableReaderOptions& table_reader_options, - unique_ptr&& file, - uint64_t file_size, - unique_ptr* table) const override; + Status NewTableReader( + const TableReaderOptions& table_reader_options, + unique_ptr&& file, uint64_t file_size, + unique_ptr* table, + bool prefetch_index_and_filter_in_cache = true) const override; TableBuilder* NewTableBuilder( const TableBuilderOptions& table_builder_options, diff --git a/table/mock_table.cc b/table/mock_table.cc index 7d5cefa78..2b311d0f8 100644 --- a/table/mock_table.cc +++ b/table/mock_table.cc @@ -59,7 +59,8 @@ MockTableFactory::MockTableFactory() : next_id_(1) {} Status MockTableFactory::NewTableReader( const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - unique_ptr* table_reader) const { + unique_ptr* table_reader, + bool prefetch_index_and_filter_in_cache) const { uint32_t id = GetIDFromFile(file.get()); MutexLock lock_guard(&file_system_.mutex); diff --git a/table/mock_table.h b/table/mock_table.h index 4352a2c7a..d9afba46f 100644 --- a/table/mock_table.h +++ b/table/mock_table.h @@ -147,10 +147,11 @@ class MockTableFactory : public TableFactory { public: MockTableFactory(); const char* Name() const override { return "MockTable"; } - Status NewTableReader(const TableReaderOptions& table_reader_options, - unique_ptr&& file, - uint64_t file_size, - unique_ptr* table_reader) const override; + Status NewTableReader( + const TableReaderOptions& table_reader_options, + unique_ptr&& file, uint64_t file_size, + unique_ptr* table_reader, + bool prefetch_index_and_filter_in_cache = true) const override; TableBuilder* NewTableBuilder( const TableBuilderOptions& table_builder_options, uint32_t column_familly_id, WritableFileWriter* file) const override; diff --git a/table/plain_table_factory.cc b/table/plain_table_factory.cc index 7db5e672c..55a68fce1 100644 --- a/table/plain_table_factory.cc +++ b/table/plain_table_factory.cc @@ -17,7 +17,8 @@ namespace rocksdb { Status PlainTableFactory::NewTableReader( const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - unique_ptr* table) const { + unique_ptr* table, + bool prefetch_index_and_filter_in_cache) const { return PlainTableReader::Open( table_reader_options.ioptions, table_reader_options.env_options, table_reader_options.internal_comparator, std::move(file), file_size, diff --git a/table/plain_table_factory.h b/table/plain_table_factory.h index 154cbff06..33cd31347 100644 --- a/table/plain_table_factory.h +++ b/table/plain_table_factory.h @@ -149,8 +149,8 @@ class PlainTableFactory : public TableFactory { const char* Name() const override { return "PlainTable"; } Status NewTableReader(const TableReaderOptions& table_reader_options, unique_ptr&& file, - uint64_t file_size, - unique_ptr* table) const override; + uint64_t file_size, unique_ptr* table, + bool prefetch_index_and_filter_in_cache) const override; TableBuilder* NewTableBuilder( const TableBuilderOptions& table_builder_options, diff --git a/utilities/options/options_util_test.cc b/utilities/options/options_util_test.cc index 55e2d6cbe..0873071ee 100644 --- a/utilities/options/options_util_test.cc +++ b/utilities/options/options_util_test.cc @@ -105,7 +105,8 @@ class DummyTableFactory : public TableFactory { virtual Status NewTableReader(const TableReaderOptions& table_reader_options, unique_ptr&& file, uint64_t file_size, - unique_ptr* table_reader) const { + unique_ptr* table_reader, + bool prefetch_index_and_filter_in_cache) const { return Status::NotSupported(); }