andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Avoid reloading filter on Get() if cache_index_and_filter_blocks == false

Browse Source 
Summary:
This fixes the case that filter policy is missing in SST file, but we
open the table with filter policy on and cache_index_and_filter_blocks =
false. The current behavior is that we will try to load it every time on
Get() but fail.

Test Plan: unit test

Reviewers: yhchiang, igor, rven, sdong

Reviewed By: sdong

Subscribers: leveldb

Differential Revision: https://reviews.facebook.net/D25455
This commit is contained in:
Lei Jin 2014-10-22 11:52:35 -07:00
parent e11a5e776f
commit 0fd985f427
4 changed files with 87 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
table/block_based_table_factory.cc
View File

@ -71,6 +71,11 @@ Status BlockBasedTableFactory::SanitizeOptions(
return Status::InvalidArgument("Hash index is specified for block-based " return Status::InvalidArgument("Hash index is specified for block-based "
"table, but prefix_extractor is not given"); "table, but prefix_extractor is not given");
} }
if (table_options_.cache_index_and_filter_blocks &&
table_options_.no_block_cache) {
return Status::InvalidArgument("Enable cache_index_and_filter_blocks, "
", but block cache is disabled");
}
return Status::OK(); return Status::OK();
} }

106
table/block_based_table_reader.cc
View File

@ -483,8 +483,8 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions,
} }
// Will use block cache for index/filter blocks access? // Will use block cache for index/filter blocks access?
if (table_options.block_cache && if (table_options.cache_index_and_filter_blocks) {
table_options.cache_index_and_filter_blocks) { assert(table_options.block_cache != nullptr);
// Hack: Call NewIndexIterator() to implicitly add index to the block_cache // Hack: Call NewIndexIterator() to implicitly add index to the block_cache
unique_ptr<Iterator> iter(new_table->NewIndexIterator(ReadOptions())); unique_ptr<Iterator> iter(new_table->NewIndexIterator(ReadOptions()));
s = iter->status(); s = iter->status();
@ -506,19 +506,7 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions,
// Set filter block // Set filter block
if (rep->filter_policy) { if (rep->filter_policy) {
// First try reading full_filter, then reading block_based_filter rep->filter.reset(ReadFilter(rep, meta_iter.get(), nullptr));
for (auto filter_block_prefix : { kFullFilterBlockPrefix,
kFilterBlockPrefix }) {
std::string key = filter_block_prefix;
key.append(rep->filter_policy->Name());
BlockHandle handle;
if (FindMetaBlock(meta_iter.get(), key, &handle).ok()) {
rep->filter.reset(ReadFilter(handle, rep,
filter_block_prefix, nullptr));
break;
}
}
} }
} else { } else {
delete index_reader; delete index_reader;
@ -726,33 +714,43 @@ Status BlockBasedTable::PutDataBlockToCache(
} }
FilterBlockReader* BlockBasedTable::ReadFilter( FilterBlockReader* BlockBasedTable::ReadFilter(
const BlockHandle& filter_handle, BlockBasedTable::Rep* rep, Rep* rep, Iterator* meta_index_iter, size_t* filter_size) {
const std::string& filter_block_prefix, size_t* filter_size) {
// TODO: We might want to unify with ReadBlockFromFile() if we start // TODO: We might want to unify with ReadBlockFromFile() if we start
// requiring checksum verification in Table::Open. // requiring checksum verification in Table::Open.
ReadOptions opt; for (auto prefix : {kFullFilterBlockPrefix, kFilterBlockPrefix}) {
BlockContents block; std::string filter_block_key = prefix;
if (!ReadBlockContents(rep->file.get(), rep->footer, opt, filter_handle, filter_block_key.append(rep->filter_policy->Name());
&block, rep->ioptions.env, false).ok()) { BlockHandle handle;
return nullptr; if (FindMetaBlock(meta_index_iter, filter_block_key, &handle).ok()) {
} BlockContents block;
if (!ReadBlockContents(rep->file.get(), rep->footer, ReadOptions(),
handle, &block, rep->ioptions.env, false).ok()) {
// Error reading the block
return nullptr;
}
if (filter_size) { if (filter_size) {
*filter_size = block.data.size(); *filter_size = block.data.size();
} }
assert(rep->filter_policy); assert(rep->filter_policy);
if (kFilterBlockPrefix == filter_block_prefix) { if (kFilterBlockPrefix == prefix) {
return new BlockBasedFilterBlockReader( return new BlockBasedFilterBlockReader(
rep->ioptions.prefix_extractor, rep->table_options, std::move(block)); rep->ioptions.prefix_extractor, rep->table_options,
} else if (kFullFilterBlockPrefix == filter_block_prefix) { std::move(block));
auto filter_bits_reader = rep->filter_policy-> } else if (kFullFilterBlockPrefix == prefix) {
GetFilterBitsReader(block.data); auto filter_bits_reader = rep->filter_policy->
GetFilterBitsReader(block.data);
if (filter_bits_reader != nullptr) { if (filter_bits_reader != nullptr) {
return new FullFilterBlockReader(rep->ioptions.prefix_extractor, return new FullFilterBlockReader(rep->ioptions.prefix_extractor,
rep->table_options, std::move(block), rep->table_options,
filter_bits_reader); std::move(block),
filter_bits_reader);
}
} else {
assert(false);
return nullptr;
}
} }
} }
return nullptr; return nullptr;
@ -760,8 +758,11 @@ FilterBlockReader* BlockBasedTable::ReadFilter(
BlockBasedTable::CachableEntry<FilterBlockReader> BlockBasedTable::GetFilter( BlockBasedTable::CachableEntry<FilterBlockReader> BlockBasedTable::GetFilter(
bool no_io) const { bool no_io) const {
// filter pre-populated // If cache_index_and_filter_blocks is false, filter should be pre-populated.
if (rep_->filter != nullptr) { // We will return rep_->filter anyway. rep_->filter can be nullptr if filter
// read fails at Open() time. We don't want to reload again since it will
// most probably fail again.
if (!rep_->table_options.cache_index_and_filter_blocks) {
return {rep_->filter.get(), nullptr /* cache handle */}; return {rep_->filter.get(), nullptr /* cache handle */};
} }
@ -775,8 +776,7 @@ BlockBasedTable::CachableEntry<FilterBlockReader> BlockBasedTable::GetFilter(
char cache_key[kMaxCacheKeyPrefixSize + kMaxVarint64Length]; char cache_key[kMaxCacheKeyPrefixSize + kMaxVarint64Length];
auto key = GetCacheKey(rep_->cache_key_prefix, rep_->cache_key_prefix_size, auto key = GetCacheKey(rep_->cache_key_prefix, rep_->cache_key_prefix_size,
rep_->footer.metaindex_handle(), rep_->footer.metaindex_handle(),
cache_key cache_key);
);
Statistics* statistics = rep_->ioptions.statistics; Statistics* statistics = rep_->ioptions.statistics;
auto cache_handle = auto cache_handle =
@ -797,22 +797,12 @@ BlockBasedTable::CachableEntry<FilterBlockReader> BlockBasedTable::GetFilter(
auto s = ReadMetaBlock(rep_, &meta, &iter); auto s = ReadMetaBlock(rep_, &meta, &iter);
if (s.ok()) { if (s.ok()) {
// First try reading full_filter, then reading block_based_filter filter = ReadFilter(rep_, iter.get(), &filter_size);
for (auto filter_block_prefix : {kFullFilterBlockPrefix, if (filter != nullptr) {
kFilterBlockPrefix}) { assert(filter_size > 0);
std::string filter_block_key = filter_block_prefix; cache_handle = block_cache->Insert(
filter_block_key.append(rep_->filter_policy->Name()); key, filter, filter_size, &DeleteCachedEntry<FilterBlockReader>);
BlockHandle handle; RecordTick(statistics, BLOCK_CACHE_ADD);
if (FindMetaBlock(iter.get(), filter_block_key, &handle).ok()) {
filter = ReadFilter(handle, rep_, filter_block_prefix, &filter_size);
if (filter == nullptr) break; // err happen in ReadFilter
assert(filter_size > 0);
cache_handle = block_cache->Insert(
key, filter, filter_size, &DeleteCachedEntry<FilterBlockReader>);
RecordTick(statistics, BLOCK_CACHE_ADD);
break;
}
} }
} }
} }

8
table/block_based_table_reader.h
View File

@ -183,10 +183,10 @@ class BlockBasedTable : public TableReader {
std::unique_ptr<Iterator>* iter); std::unique_ptr<Iterator>* iter);
// Create the filter from the filter block. // Create the filter from the filter block.
static FilterBlockReader* ReadFilter(const BlockHandle& filter_handle, static FilterBlockReader* ReadFilter(
Rep* rep, Rep* rep,
const std::string& filter_block_prefix, Iterator* meta_index_iter,
size_t* filter_size = nullptr); size_t* filter_size = nullptr);
static void SetupCacheKeyPrefix(Rep* rep); static void SetupCacheKeyPrefix(Rep* rep);

53
table/table_test.cc
View File

@ -1461,8 +1461,6 @@ TEST(BlockBasedTableTest, BlockCacheDisabledTest) {
options.create_if_missing = true; options.create_if_missing = true;
options.statistics = CreateDBStatistics(); options.statistics = CreateDBStatistics();
BlockBasedTableOptions table_options; BlockBasedTableOptions table_options;
// Intentionally commented out: table_options.cache_index_and_filter_blocks =
// true;
table_options.block_cache = NewLRUCache(1024); table_options.block_cache = NewLRUCache(1024);
table_options.filter_policy.reset(NewBloomFilterPolicy(10)); table_options.filter_policy.reset(NewBloomFilterPolicy(10));
options.table_factory.reset(new BlockBasedTableFactory(table_options)); options.table_factory.reset(new BlockBasedTableFactory(table_options));
@ -1521,7 +1519,7 @@ TEST(BlockBasedTableTest, FilterBlockInBlockCache) {
c.Finish(options, ioptions, table_options, c.Finish(options, ioptions, table_options,
GetPlainInternalComparator(options.comparator), &keys, &kvmap); GetPlainInternalComparator(options.comparator), &keys, &kvmap);
// preloading filter/index blocks is prohibited. // preloading filter/index blocks is prohibited.
auto reader = dynamic_cast<BlockBasedTable*>(c.GetTableReader()); auto* reader = dynamic_cast<BlockBasedTable*>(c.GetTableReader());
ASSERT_TRUE(!reader->TEST_filter_block_preloaded()); ASSERT_TRUE(!reader->TEST_filter_block_preloaded());
ASSERT_TRUE(!reader->TEST_index_reader_preloaded()); ASSERT_TRUE(!reader->TEST_index_reader_preloaded());
@ -1567,28 +1565,11 @@ TEST(BlockBasedTableTest, FilterBlockInBlockCache) {
// release the iterator so that the block cache can reset correctly. // release the iterator so that the block cache can reset correctly.
iter.reset(); iter.reset();
// -- PART 2: Open without block cache // -- PART 2: Open with very small block cache
table_options.no_block_cache = true;
table_options.block_cache.reset();
options.table_factory.reset(new BlockBasedTableFactory(table_options));
options.statistics = CreateDBStatistics(); // reset the stats
const ImmutableCFOptions ioptions1(options);
c.Reopen(ioptions1);
table_options.no_block_cache = false;
{
iter.reset(c.NewIterator());
iter->SeekToFirst();
ASSERT_EQ("key", iter->key().ToString());
BlockCachePropertiesSnapshot props(options.statistics.get());
// Nothing is affected at all
props.AssertEqual(0, 0, 0, 0);
}
// -- PART 3: Open with very small block cache
// In this test, no block will ever get hit since the block cache is // In this test, no block will ever get hit since the block cache is
// too small to fit even one entry. // too small to fit even one entry.
table_options.block_cache = NewLRUCache(1); table_options.block_cache = NewLRUCache(1);
options.statistics = CreateDBStatistics();
options.table_factory.reset(new BlockBasedTableFactory(table_options)); options.table_factory.reset(new BlockBasedTableFactory(table_options));
const ImmutableCFOptions ioptions2(options); const ImmutableCFOptions ioptions2(options);
c.Reopen(ioptions2); c.Reopen(ioptions2);
@ -1598,7 +1579,6 @@ TEST(BlockBasedTableTest, FilterBlockInBlockCache) {
0, 0, 0); 0, 0, 0);
} }
{ {
// Both index and data block get accessed. // Both index and data block get accessed.
// It first cache index block then data block. But since the cache size // It first cache index block then data block. But since the cache size
@ -1618,6 +1598,33 @@ TEST(BlockBasedTableTest, FilterBlockInBlockCache) {
props.AssertEqual(2, 0, 0 + 1, // data block miss props.AssertEqual(2, 0, 0 + 1, // data block miss
0); 0);
} }
iter.reset();
// -- PART 3: Open table with bloom filter enabled but not in SST file
table_options.block_cache = NewLRUCache(4096);
table_options.cache_index_and_filter_blocks = false;
options.table_factory.reset(NewBlockBasedTableFactory(table_options));
TableConstructor c3(BytewiseComparator());
c3.Add("k01", "hello");
ImmutableCFOptions ioptions3(options);
// Generate table without filter policy
c3.Finish(options, ioptions3, table_options,
GetPlainInternalComparator(options.comparator), &keys, &kvmap);
// Open table with filter policy
table_options.filter_policy.reset(NewBloomFilterPolicy(1));
options.table_factory.reset(new BlockBasedTableFactory(table_options));
options.statistics = CreateDBStatistics();
ImmutableCFOptions ioptions4(options);
ASSERT_OK(c3.Reopen(ioptions4));
reader = dynamic_cast<BlockBasedTable*>(c3.GetTableReader());
ASSERT_TRUE(!reader->TEST_filter_block_preloaded());
GetContext get_context(options.comparator, nullptr, nullptr, nullptr,
GetContext::kNotFound, Slice(), nullptr,
nullptr, nullptr);
ASSERT_OK(reader->Get(ReadOptions(), "k01", &get_context));
BlockCachePropertiesSnapshot props(options.statistics.get());
props.AssertFilterBlockStat(0, 0);
} }
TEST(BlockBasedTableTest, BlockCacheLeak) { TEST(BlockBasedTableTest, BlockCacheLeak) {