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

@@ -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.

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,6 +190,9 @@ Status PlainTableBuilder::Finish() {
 
   if (store_index_in_file_ && (properties_.num_entries > 0)) {
     assert(properties_.num_entries <= std::numeric_limits<uint32_t>::max());
+    Status s;
+    BlockHandle bloom_block_handle;
+    if (bloom_bits_per_key_ > 0) {
       bloom_block_.SetTotalBits(
           &arena_,
           static_cast<uint32_t>(properties_.num_entries) * bloom_bits_per_key_,
@@ -201,27 +203,27 @@ Status PlainTableBuilder::Finish() {
                   bloom_block_.GetNumBlocks());
 
       bloom_block_.AddKeysHashes(keys_or_prefixes_hashes_);
-    BlockHandle bloom_block_handle;
-    auto finish_result = bloom_block_.Finish();
 
-    properties_.filter_size = finish_result.size();
+      Slice bloom_finish_result = bloom_block_.Finish();
-    auto s = WriteBlock(finish_result, file_, &offset_, &bloom_block_handle);
+
+      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();
+    properties_.index_size = index_finish_result.size();
-    s = WriteBlock(finish_result, file_, &offset_, &index_block_handle);
+    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);
   }

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();
 
-  Slice* bloom_block;
+  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 (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<unsigned char*>(
             reinterpret_cast<const unsigned char*>(bloom_block->data())),
         static_cast<uint32_t>(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,