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

Fix bug in KeyMayExist

Browse Source 
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
This commit is contained in:
Mayank Agarwal 2013-08-25 13:40:02 -07:00
parent b1074ac24f
commit dad2731729
2 changed files with 2 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
db/db_test.cc
View File

@ -843,8 +843,7 @@ TEST(DBTest, KeyMayExist) {
value.clear(); value.clear();
value_found = false; value_found = false;
ASSERT_TRUE(db_->KeyMayExist(ropts, "a", &value, &value_found)); ASSERT_TRUE(db_->KeyMayExist(ropts, "a", &value, &value_found));
ASSERT_TRUE(value_found); ASSERT_TRUE(!value_found);
ASSERT_EQ("b", value);
ASSERT_OK(db_->Delete(WriteOptions(), "a")); ASSERT_OK(db_->Delete(WriteOptions(), "a"));
ASSERT_TRUE(!db_->KeyMayExist(ropts, "a", &value)); ASSERT_TRUE(!db_->KeyMayExist(ropts, "a", &value));

2
table/table.cc
View File

@ -421,7 +421,7 @@ Status Table::InternalGet(const ReadOptions& options, const Slice& k,
} else { } else {
bool didIO = false; bool didIO = false;
Iterator* block_iter = BlockReader(this, options, iiter->value(), 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 if (no_io && !block_iter) { // couldn't get block from block_cache
// Update Saver.state to Found because we are only looking for whether // Update Saver.state to Found because we are only looking for whether