From a4eb7387b29368347967a6efab0e2acfe4d5642c Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Thu, 17 Nov 2016 10:29:35 -0800 Subject: [PATCH] Allow plain table to store index on file with bloom filter disabled Summary: Currently plain table bloom filter is required if storing metadata on file. Remove the constraint. Closes https://github.com/facebook/rocksdb/pull/1525 Differential Revision: D4190977 Pulled By: siying fbshipit-source-id: be60442 --- db/plain_table_db_test.cc | 4 ---- table/plain_table_builder.cc | 44 +++++++++++++++++++----------------- table/plain_table_reader.cc | 30 ++++++++++++++---------- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/db/plain_table_db_test.cc b/db/plain_table_db_test.cc index b71da7f1a..145ebdf08 100644 --- a/db/plain_table_db_test.cc +++ b/db/plain_table_db_test.cc @@ -387,10 +387,6 @@ TEST_P(PlainTableDBTest, Flush) { for (int total_order = 0; total_order <= 1; total_order++) { for (int store_index_in_file = 0; store_index_in_file <= 1; ++store_index_in_file) { - if (!bloom_bits && store_index_in_file) { - continue; - } - Options options = CurrentOptions(); options.create_if_missing = true; // Set only one bucket to force bucket conflict. diff --git a/table/plain_table_builder.cc b/table/plain_table_builder.cc index fa3c7874e..e0428f19e 100644 --- a/table/plain_table_builder.cc +++ b/table/plain_table_builder.cc @@ -80,7 +80,6 @@ PlainTableBuilder::PlainTableBuilder( index_builder_.reset( new PlainTableIndexBuilder(&arena_, ioptions, index_sparseness, hash_table_ratio, huge_page_tlb_size_)); - assert(bloom_bits_per_key_ > 0); properties_.user_collected_properties [PlainTablePropertyNames::kBloomVersion] = "1"; // For future use } @@ -191,37 +190,40 @@ Status PlainTableBuilder::Finish() { if (store_index_in_file_ && (properties_.num_entries > 0)) { assert(properties_.num_entries <= std::numeric_limits::max()); - bloom_block_.SetTotalBits( - &arena_, - static_cast(properties_.num_entries) * bloom_bits_per_key_, - ioptions_.bloom_locality, huge_page_tlb_size_, ioptions_.info_log); - - PutVarint32(&properties_.user_collected_properties - [PlainTablePropertyNames::kNumBloomBlocks], - bloom_block_.GetNumBlocks()); - - bloom_block_.AddKeysHashes(keys_or_prefixes_hashes_); + Status s; BlockHandle bloom_block_handle; - auto finish_result = bloom_block_.Finish(); + if (bloom_bits_per_key_ > 0) { + bloom_block_.SetTotalBits( + &arena_, + static_cast(properties_.num_entries) * bloom_bits_per_key_, + ioptions_.bloom_locality, huge_page_tlb_size_, ioptions_.info_log); - properties_.filter_size = finish_result.size(); - auto s = WriteBlock(finish_result, file_, &offset_, &bloom_block_handle); + PutVarint32(&properties_.user_collected_properties + [PlainTablePropertyNames::kNumBloomBlocks], + bloom_block_.GetNumBlocks()); - if (!s.ok()) { - return s; + bloom_block_.AddKeysHashes(keys_or_prefixes_hashes_); + + Slice bloom_finish_result = bloom_block_.Finish(); + + properties_.filter_size = bloom_finish_result.size(); + s = WriteBlock(bloom_finish_result, file_, &offset_, &bloom_block_handle); + + if (!s.ok()) { + return s; + } + meta_index_builer.Add(BloomBlockBuilder::kBloomBlock, bloom_block_handle); } - BlockHandle index_block_handle; - finish_result = index_builder_->Finish(); + Slice index_finish_result = index_builder_->Finish(); - properties_.index_size = finish_result.size(); - s = WriteBlock(finish_result, file_, &offset_, &index_block_handle); + properties_.index_size = index_finish_result.size(); + s = WriteBlock(index_finish_result, file_, &offset_, &index_block_handle); if (!s.ok()) { return s; } - meta_index_builer.Add(BloomBlockBuilder::kBloomBlock, bloom_block_handle); meta_index_builer.Add(PlainTableIndexBuilder::kPlainTableIndexBlock, index_block_handle); } diff --git a/table/plain_table_reader.cc b/table/plain_table_reader.cc index e46a33a40..cac59bd05 100644 --- a/table/plain_table_reader.cc +++ b/table/plain_table_reader.cc @@ -294,21 +294,25 @@ Status PlainTableReader::PopulateIndex(TableProperties* props, assert(props != nullptr); table_properties_.reset(props); - BlockContents bloom_block_contents; - auto s = ReadMetaBlock(file_info_.file.get(), file_size_, - kPlainTableMagicNumber, ioptions_, - BloomBlockBuilder::kBloomBlock, &bloom_block_contents); - bool index_in_file = s.ok(); - BlockContents index_block_contents; - s = ReadMetaBlock( + Status s = ReadMetaBlock( file_info_.file.get(), file_size_, kPlainTableMagicNumber, ioptions_, PlainTableIndexBuilder::kPlainTableIndexBlock, &index_block_contents); - index_in_file &= s.ok(); + bool index_in_file = s.ok(); + + BlockContents bloom_block_contents; + bool bloom_in_file = false; + // We only need to read the bloom block if index block is in file. + if (index_in_file) { + s = ReadMetaBlock(file_info_.file.get(), file_size_, kPlainTableMagicNumber, + ioptions_, BloomBlockBuilder::kBloomBlock, + &bloom_block_contents); + bloom_in_file = s.ok() && bloom_block_contents.data.size() > 0; + } Slice* bloom_block; - if (index_in_file) { + if (bloom_in_file) { // If bloom_block_contents.allocation is not empty (which will be the case // for non-mmap mode), it holds the alloated memory for the bloom block. // It needs to be kept alive to keep `bloom_block` valid. @@ -318,8 +322,6 @@ Status PlainTableReader::PopulateIndex(TableProperties* props, bloom_block = nullptr; } - // index_in_file == true only if there are kBloomBlock and - // kPlainTableIndexBlock in file Slice* index_block; if (index_in_file) { // If index_block_contents.allocation is not empty (which will be the case @@ -355,7 +357,7 @@ Status PlainTableReader::PopulateIndex(TableProperties* props, huge_page_tlb_size, ioptions_.info_log); } } - } else { + } else if (bloom_in_file) { enable_bloom_ = true; auto num_blocks_property = props->user_collected_properties.find( PlainTablePropertyNames::kNumBloomBlocks); @@ -372,6 +374,10 @@ Status PlainTableReader::PopulateIndex(TableProperties* props, const_cast( reinterpret_cast(bloom_block->data())), static_cast(bloom_block->size()) * 8, num_blocks); + } else { + // Index in file but no bloom in file. Disable bloom filter in this case. + enable_bloom_ = false; + bloom_bits_per_key = 0; } PlainTableIndexBuilder index_builder(&arena_, ioptions_, index_sparseness,