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

Fixed CompactionFilter::Decision::kRemoveAndSkipUntil

Browse Source 
Summary:
Embarassingly enough, the first time I tried to use my new feature in logdevice it crashed with this assertion failure:

  db/pinned_iterators_manager.h:30: void rocksdb::PinnedIteratorsManager::StartPinning(): Assertion `pinning_enabled == false' failed

The issue was that `pinned_iters_mgr_.StartPinning()` was called but `pinned_iters_mgr_.ReleasePinnedData()` wasn't.
Closes https://github.com/facebook/rocksdb/pull/1611

Differential Revision: D4265622

Pulled By: al13n321

fbshipit-source-id: 747b10f
This commit is contained in:
Mike Kolupaev 2016-12-05 15:07:01 -08:00 committed by Facebook Github Bot
parent 67f37cf198
commit beb36d9c1e
2 changed files with 9 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
db/compaction_iterator.cc
View File

@ -437,9 +437,7 @@ void CompactionIterator::NextFromInput() {
bottommost_level_); bottommost_level_);
merge_out_iter_.SeekToFirst(); merge_out_iter_.SeekToFirst();
if (merge_helper_->FilteredUntil(&skip_until)) { if (merge_out_iter_.Valid()) {
need_skip = true;
} else if (merge_out_iter_.Valid()) {
// NOTE: key, value, and ikey_ refer to old entries. // NOTE: key, value, and ikey_ refer to old entries.
// These will be correctly set below. // These will be correctly set below.
key_ = merge_out_iter_.key(); key_ = merge_out_iter_.key();
@ -460,6 +458,10 @@ void CompactionIterator::NextFromInput() {
// coming after the merges // coming after the merges
has_current_user_key_ = false; has_current_user_key_ = false;
pinned_iters_mgr_.ReleasePinnedData(); pinned_iters_mgr_.ReleasePinnedData();
if (merge_helper_->FilteredUntil(&skip_until)) {
need_skip = true;
}
} }
} else { } else {
// 1. new user key -OR- // 1. new user key -OR-

8
db/compaction_iterator_test.cc
View File

@ -272,8 +272,8 @@ TEST_F(CompactionIteratorTest, CompactionFilterSkipUntil) {
return Decision::kKeep; return Decision::kKeep;
} }
if (k == "i") { if (k == "i") {
EXPECT_EQ(ValueType::kValue, t); EXPECT_EQ(ValueType::kMergeOperand, t);
EXPECT_EQ("iv95", v); EXPECT_EQ("im95", v);
*skip_until = "z"; *skip_until = "z";
return Decision::kRemoveAndSkipUntil; return Decision::kRemoveAndSkipUntil;
} }
@ -299,10 +299,10 @@ TEST_F(CompactionIteratorTest, CompactionFilterSkipUntil) {
test::KeyStr("f", 30, kTypeMerge), // skip to "g+" test::KeyStr("f", 30, kTypeMerge), // skip to "g+"
test::KeyStr("f", 25, kTypeValue), test::KeyStr("g", 90, kTypeValue), test::KeyStr("f", 25, kTypeValue), test::KeyStr("g", 90, kTypeValue),
test::KeyStr("h", 91, kTypeValue), // keep test::KeyStr("h", 91, kTypeValue), // keep
test::KeyStr("i", 95, kTypeValue), // skip to "z" test::KeyStr("i", 95, kTypeMerge), // skip to "z"
test::KeyStr("j", 99, kTypeValue)}, test::KeyStr("j", 99, kTypeValue)},
{"av50", "am45", "bv60", "bv40", "cv35", "dm70", "em71", "fm65", "fm30", {"av50", "am45", "bv60", "bv40", "cv35", "dm70", "em71", "fm65", "fm30",
"fv25", "gv90", "hv91", "iv95", "jv99"}, "fv25", "gv90", "hv91", "im95", "jv99"},
{}, {}, kMaxSequenceNumber, &merge_op, &filter); {}, {}, kMaxSequenceNumber, &merge_op, &filter);
// Compaction should output just "a", "e" and "h" keys. // Compaction should output just "a", "e" and "h" keys.