From b4243e5a3ddf89fb197ae938da19e802cd2ddd66 Mon Sep 17 00:00:00 2001 From: Haobo Xu Date: Wed, 17 Apr 2013 21:30:21 -0700 Subject: [PATCH] [RocksDB] CompactionFilter cleanup Summary: - removed the compaction_filter_value from the callback interface. Restrict compaction filter to purging values. - modify some comments to reflect curent status. Test Plan: make check Reviewers: dhruba Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D10335 --- db/db_impl.cc | 23 ++++++++++++++--------- db/db_test.cc | 12 ++++++++---- include/leveldb/options.h | 19 +++++++++++-------- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 7f2cb033c..5d05aa981 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1590,6 +1590,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) { SequenceNumber last_sequence_for_key __attribute__((unused)) = kMaxSequenceNumber; SequenceNumber visible_in_snapshot = kMaxSequenceNumber; + std::string compaction_filter_value; for (; input->Valid() && !shutting_down_.Acquire_Load(); ) { // Prioritize immutable compaction work if (imm_.imm_flush_needed.NoBarrier_Load() != nullptr) { @@ -1605,7 +1606,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) { Slice key = input->key(); Slice value = input->value(); - Slice* compaction_filter_value = nullptr; + if (compact->compaction->ShouldStopBefore(key) && compact->builder != nullptr) { status = FinishCompactionOutputFile(compact, input.get()); @@ -1664,20 +1665,24 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) { ikey.type != kTypeDeletion && ikey.sequence < earliest_snapshot) { // If the user has specified a compaction filter, then invoke - // it. If this key is not visible via any snapshot and the - // return value of the compaction filter is true and then + // it. If the return value of the compaction filter is true, // drop this key from the output. + bool value_changed = false; + compaction_filter_value.clear(); drop = options_.CompactionFilter(options_.compaction_filter_args, - compact->compaction->level(), - ikey.user_key, value, &compaction_filter_value); - + compact->compaction->level(), + ikey.user_key, value, + &compaction_filter_value, + &value_changed); + // Another example of statistics update without holding the lock + // TODO: clean it up if (drop) { RecordTick(options_.statistics, COMPACTION_KEY_DROP_USER); } + // If the application wants to change the value, then do so here. - if (compaction_filter_value != nullptr) { - value = *compaction_filter_value; - delete compaction_filter_value; + if (value_changed) { + value = compaction_filter_value; } } diff --git a/db/db_test.cc b/db/db_test.cc index fa61ebcdc..550b221c5 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -1351,22 +1351,26 @@ TEST(DBTest, RepeatedWritesToSameKey) { static int cfilter_count; static std::string NEW_VALUE = "NewValue"; static bool keep_filter(void* arg, int level, const Slice& key, - const Slice& value, Slice** new_value) { + const Slice& value, std::string* new_value, + bool* value_changed) { assert(arg == nullptr); cfilter_count++; return false; } static bool delete_filter(void*argv, int level, const Slice& key, - const Slice& value, Slice** new_value) { + const Slice& value, std::string* new_value, + bool* value_changed) { assert(argv == nullptr); cfilter_count++; return true; } static bool change_filter(void*argv, int level, const Slice& key, - const Slice& value, Slice** new_value) { + const Slice& value, std::string* new_value, + bool* value_changed) { assert(argv == (void*)100); assert(new_value != nullptr); - *new_value = new Slice(NEW_VALUE); + *new_value = NEW_VALUE; + *value_changed = true; return false; } diff --git a/include/leveldb/options.h b/include/leveldb/options.h index 863cf919a..346f173d8 100644 --- a/include/leveldb/options.h +++ b/include/leveldb/options.h @@ -356,22 +356,25 @@ struct Options { // This method allows an application to modify/delete a key-value at // the time of compaction. The compaction process invokes this - // method for every kv that is being compacted. A return value + // method for kv that is being compacted. A return value // of false indicates that the kv should be preserved in the // output of this compaction run and a return value of true // indicates that this key-value should be removed from the // output of the compaction. The application can inspect - // the existing value of the key, modify it if needed and - // return back the new value for this key. The application - // should allocate memory for the Slice object that is used to - // return the new value and the leveldb framework will - // free up that memory. + // the existing value of the key and make decision based on it. + + // When the value is to be preserved, the application has the option + // to modify the existing_value and pass it back through new_value. + // value_changed needs to be set to true in this case. + // The compaction_filter_args, if specified here, are passed // back to the invocation of the CompactionFilter. void* compaction_filter_args; bool (*CompactionFilter)(void* compaction_filter_args, - int level, const Slice& key, - const Slice& existing_value, Slice** new_value); + int level, const Slice& key, + const Slice& existing_value, + std::string* new_value, + bool* value_changed); // Disable automatic compactions. Manual compactions can still // be issued on this database.