Fixed bug in compaction iterator

Summary:
During the refactoring, the condition that makes sure that compaction
filters are only applied to records newer than the latest snapshot
got butchered. This patch fixes the condition and adds a test case.

Test Plan: make db_compaction_filter_test && ./db_compaction_filter_test

Reviewers: rven, anthony, yhchiang, sdong, aekmekji, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D46707
This commit is contained in:
Andres Noetzli 2015-09-11 10:13:49 -07:00 committed by Islam AbdelRahman
parent af7cdbf647
commit 7584091c7f
2 changed files with 36 additions and 1 deletions

View File

@ -128,7 +128,7 @@ void CompactionIterator::NextFromInput() {
current_user_key_snapshot_ = 0; current_user_key_snapshot_ = 0;
// apply the compaction filter to the first occurrence of the user key // apply the compaction filter to the first occurrence of the user key
if (compaction_filter_ != nullptr && ikey_.type == kTypeValue && if (compaction_filter_ != nullptr && ikey_.type == kTypeValue &&
(visible_at_tip_ || latest_snapshot_)) { (visible_at_tip_ || ikey_.sequence > latest_snapshot_)) {
// If the user has specified a compaction filter and the sequence // If the user has specified a compaction filter and the sequence
// number is greater than any external snapshot, then invoke the // number is greater than any external snapshot, then invoke the
// filter. If the return value of the compaction filter is true, // filter. If the return value of the compaction filter is true,

View File

@ -537,6 +537,41 @@ TEST_F(DBTestCompactionFilter, CompactionFilterContextManual) {
} }
} }
// Compaction filters should only be applied to records that are newer than the
// latest snapshot. This test inserts records and applies a delete filter.
TEST_F(DBTestCompactionFilter, CompactionFilterSnapshot) {
Options options;
options.compaction_filter_factory = std::make_shared<DeleteFilterFactory>();
options.disable_auto_compactions = true;
options.create_if_missing = true;
options = CurrentOptions(options);
DestroyAndReopen(options);
// Put some data.
const Snapshot* snapshot;
for (int table = 0; table < 4; ++table) {
for (int i = 0; i < 10; ++i) {
Put(ToString(table * 100 + i), "val");
}
Flush();
if (table == 0) {
snapshot = db_->GetSnapshot();
}
}
cfilter_count = 0;
ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
// The filter should delete 10 records.
ASSERT_EQ(30U, cfilter_count);
// Release the snapshot and compact again -> now all records should be
// removed.
db_->ReleaseSnapshot(snapshot);
ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));
ASSERT_EQ(0U, CountLiveFiles());
}
} // namespace rocksdb } // namespace rocksdb
int main(int argc, char** argv) { int main(int argc, char** argv) {