From d87cffaea46d282defe570530ae3bedbf3ea00cf Mon Sep 17 00:00:00 2001 From: sdong Date: Fri, 17 Jan 2020 01:39:22 -0800 Subject: [PATCH] Fix another bug caused by recent hash index fix (#6305) Summary: Recent bug fix related to hash index introduced a new bug: hash index can return NotFound but it is not handled by BlockBasedTable::Get(). The end result is that Get() stops being executed too early. Fix it by ignoring NotFound code in Get(). Pull Request resolved: https://github.com/facebook/rocksdb/pull/6305 Test Plan: A problematic DB used to return NotFound incorrectly, and now able to return correct result. Will try to construct a unit test too.0 Differential Revision: D19438925 fbshipit-source-id: e751afa8c13728d56511cfeb1bc811ecb99f3217 --- db/db_test2.cc | 42 +++++++++++++++++++ table/block_based/block_based_table_reader.cc | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/db/db_test2.cc b/db/db_test2.cc index e74344410..7ff8cd796 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -4362,6 +4362,48 @@ TEST_F(DBTest2, BlockBasedTablePrefixIndexSeekForPrev) { } } +TEST_F(DBTest2, BlockBasedTablePrefixGetIndexNotFound) { + // create a DB with block prefix index + BlockBasedTableOptions table_options; + Options options = CurrentOptions(); + table_options.block_size = 300; + table_options.index_type = BlockBasedTableOptions::kHashSearch; + table_options.index_shortening = + BlockBasedTableOptions::IndexShorteningMode::kNoShortening; + options.table_factory.reset(NewBlockBasedTableFactory(table_options)); + options.prefix_extractor.reset(NewFixedPrefixTransform(1)); + options.level0_file_num_compaction_trigger = 8; + + Reopen(options); + + ASSERT_OK(Put("b1", "ok")); + Flush(); + + // Flushing several files so that the chance that hash bucket + // is empty fo "b" in at least one of the files is high. + ASSERT_OK(Put("a1", "")); + ASSERT_OK(Put("c1", "")); + Flush(); + + ASSERT_OK(Put("a2", "")); + ASSERT_OK(Put("c2", "")); + Flush(); + + ASSERT_OK(Put("a3", "")); + ASSERT_OK(Put("c3", "")); + Flush(); + + ASSERT_OK(Put("a4", "")); + ASSERT_OK(Put("c4", "")); + Flush(); + + ASSERT_OK(Put("a5", "")); + ASSERT_OK(Put("c5", "")); + Flush(); + + ASSERT_EQ("ok", Get("b1")); +} + } // namespace rocksdb #ifdef ROCKSDB_UNITTESTS_WITH_CUSTOM_OBJECTS_FROM_STATIC_LIBS diff --git a/table/block_based/block_based_table_reader.cc b/table/block_based/block_based_table_reader.cc index dca08d2f4..7adc2277c 100644 --- a/table/block_based/block_based_table_reader.cc +++ b/table/block_based/block_based_table_reader.cc @@ -3467,7 +3467,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_full_true_positive, 1, rep_->level); } - if (s.ok()) { + if (s.ok() && !iiter->status().IsNotFound()) { s = iiter->status(); } }