From dad2731729dd166b3bfae01c898700653355c451 Mon Sep 17 00:00:00 2001 From: Mayank Agarwal Date: Sun, 25 Aug 2013 13:40:02 -0700 Subject: [PATCH] Fix bug in KeyMayExist Summary: In KeyMayExist.db_test we do a Flush which causes sst file to be written and added as open file in TableCache, but block cache for the file is not populated. So value_found should have been false where it was true and KeyMayExist.db_test should not have passed earlier. But it passed because BlockReader in table/table.cc takes 2 default arguments at the end called for_compaction and no_io. Although I passed no_io=true from InternalGet to BlockReader, but it understood for_compaction=true and defaulted no_io to false. This is a bug and although will be removed by Dhruba's new patch to incorporate no_io in readoptions, I'm submitting this patch to fix this bug independently of that patch. Test Plan: make all check Reviewers: dhruba, haobo Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D12537 --- db/db_test.cc | 3 +-- table/table.cc | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/db/db_test.cc b/db/db_test.cc index 3f61b23944..faa24c6bac 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -843,8 +843,7 @@ TEST(DBTest, KeyMayExist) { value.clear(); value_found = false; ASSERT_TRUE(db_->KeyMayExist(ropts, "a", &value, &value_found)); - ASSERT_TRUE(value_found); - ASSERT_EQ("b", value); + ASSERT_TRUE(!value_found); ASSERT_OK(db_->Delete(WriteOptions(), "a")); ASSERT_TRUE(!db_->KeyMayExist(ropts, "a", &value)); diff --git a/table/table.cc b/table/table.cc index 972c88080f..6d7ddb6ac2 100644 --- a/table/table.cc +++ b/table/table.cc @@ -421,7 +421,7 @@ Status Table::InternalGet(const ReadOptions& options, const Slice& k, } else { bool didIO = false; Iterator* block_iter = BlockReader(this, options, iiter->value(), - &didIO, no_io); + &didIO, false, no_io); if (no_io && !block_iter) { // couldn't get block from block_cache // Update Saver.state to Found because we are only looking for whether