From 1c9f8f0884b0b7ee1cf01a9ae42e32049d94b8a8 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Thu, 27 Mar 2014 08:22:59 -0700 Subject: [PATCH] Fix valgrind issues Summary: NewFixedPrefixTransform is leaked in default options. Broken by https://github.com/facebook/rocksdb/commit/b47812fba601e23872349407d565d15f0b41a2fe Also included in the diff some code cleanup Test Plan: valgrind env_test also make check Reviewers: haobo, danguo, yhchiang Reviewed By: danguo CC: leveldb Differential Revision: https://reviews.facebook.net/D17211 --- db/db_impl.cc | 2 -- include/rocksdb/compaction_filter.h | 6 +++--- include/rocksdb/options.h | 2 ++ util/options.cc | 4 +--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 9cc957f85..2c24df79d 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -70,7 +70,6 @@ namespace rocksdb { int DBImpl::SuperVersion::dummy = 0; void* const DBImpl::SuperVersion::kSVInUse = &DBImpl::SuperVersion::dummy; void* const DBImpl::SuperVersion::kSVObsolete = nullptr; -const std::string kNullString = "NULL"; void DumpLeveldbBuildVersion(Logger * log); @@ -2897,7 +2896,6 @@ Status DBImpl::DoCompactionWork(CompactionState* compact, assert(compact); compact->CleanupBatchBuffer(); compact->CleanupMergedBuffer(); - compact->cur_prefix_ = kNullString; bool prefix_initialized = false; int64_t imm_micros = 0; // Micros spent doing imm_ compactions diff --git a/include/rocksdb/compaction_filter.h b/include/rocksdb/compaction_filter.h index 9576bf2ca..f54ee620c 100644 --- a/include/rocksdb/compaction_filter.h +++ b/include/rocksdb/compaction_filter.h @@ -139,6 +139,7 @@ class DefaultCompactionFilterFactory : public CompactionFilterFactory { // class CompactionFilterFactoryV2 { public: + // NOTE: CompactionFilterFactoryV2 will not delete prefix_extractor explicit CompactionFilterFactoryV2(const SliceTransform* prefix_extractor) : prefix_extractor_(prefix_extractor) { } @@ -169,9 +170,8 @@ class CompactionFilterFactoryV2 { // return any filter class DefaultCompactionFilterFactoryV2 : public CompactionFilterFactoryV2 { public: - explicit DefaultCompactionFilterFactoryV2( - const SliceTransform* prefix_extractor) - : CompactionFilterFactoryV2(prefix_extractor) { } + explicit DefaultCompactionFilterFactoryV2() + : CompactionFilterFactoryV2(nullptr) { } virtual std::unique_ptr CreateCompactionFilterV2( diff --git a/include/rocksdb/options.h b/include/rocksdb/options.h index 28b2d58bc..b7723ff59 100644 --- a/include/rocksdb/options.h +++ b/include/rocksdb/options.h @@ -126,6 +126,8 @@ struct Options { // Version TWO of the compaction_filter_factory // It supports rolling compaction + // + // Default: a factory that doesn't provide any object std::shared_ptr compaction_filter_factory_v2; // If true, the database will be created if it is missing. diff --git a/util/options.cc b/util/options.cc index bc325e9c0..7997aa969 100644 --- a/util/options.cc +++ b/util/options.cc @@ -32,9 +32,7 @@ Options::Options() compaction_filter(nullptr), compaction_filter_factory(std::shared_ptr( new DefaultCompactionFilterFactory())), - compaction_filter_factory_v2( - new DefaultCompactionFilterFactoryV2( - NewFixedPrefixTransform(8))), + compaction_filter_factory_v2(new DefaultCompactionFilterFactoryV2()), create_if_missing(false), error_if_exists(false), paranoid_checks(false),