[RocksDB] Fix TransformRepFactory related valgrind problem

Summary: Let TransformRepFactory own the passed in transform. Also make it better encapsulated.

Test Plan: make valgrind_check;

Reviewers: dhruba, emayanke

Reviewed By: emayanke

CC: leveldb

Differential Revision: https://reviews.facebook.net/D12591

db/db_impl.cc

@@ -169,7 +169,8 @@ Options SanitizeOptions(const std::string& dbname,
     // function.
     auto factory = dynamic_cast<PrefixHashRepFactory*>(
       result.memtable_factory.get());
-    if (factory != nullptr && factory->transform_ != result.prefix_extractor) {
+    if (factory != nullptr &&
+        factory->GetTransform() != result.prefix_extractor) {
       Log(result.info_log, "A prefix hash representation factory was supplied "
           "whose prefix extractor does not match options.prefix_extractor. "
           "Falling back to skip list representation factory");

db/db_test.cc

@@ -3932,7 +3932,6 @@ TEST(DBTest, PrefixScan) {
   ASSERT_EQ(env_->random_read_counter_.Read(), 11);
   Close();
   delete options.filter_policy;
-  delete options.prefix_extractor;
 }
 
 std::string MakeKey(unsigned int num) {

include/rocksdb/memtablerep.h

@@ -181,7 +181,7 @@ public:
 //
 // Parameters:
 //   transform: The SliceTransform to bucket user keys on. TransformRepFactory
-//     assumes it does not own the pointer.
+//     owns the pointer.
 //   bucket_count: Passed to the constructor of the underlying
 //     std::unordered_map of each TransformRep. On initialization, the
 //     underlying array will be at least bucket_count size.
@@ -189,17 +189,24 @@ public:
 //     hashed onto a read-write lock which controls access to that lock. More
 //     locks means finer-grained concurrency but more memory overhead.
 class TransformRepFactory : public MemTableRepFactory {
-public:
-  const SliceTransform* transform_;
-  const size_t bucket_count_;
-  const size_t num_locks_;
+ public:
   explicit TransformRepFactory(const SliceTransform* transform,
     size_t bucket_count, size_t num_locks = 1000)
     : transform_(transform),
       bucket_count_(bucket_count),
       num_locks_(num_locks) { }
+
+  virtual ~TransformRepFactory() { delete transform_; }
+
   virtual std::shared_ptr<MemTableRep> CreateMemTableRep(
     MemTableRep::KeyComparator&, Arena*) override;
+
+  const SliceTransform* GetTransform() { return transform_; }
+
+ protected:
+  const SliceTransform* transform_;
+  const size_t bucket_count_;
+  const size_t num_locks_;
 };
 
 // UnsortedReps bin user keys based on an identity function transform -- that
@@ -207,13 +214,11 @@ public:
 //
 // Parameters: See TransformRepFactory.
 class UnsortedRepFactory : public TransformRepFactory {
-  const SliceTransform* transform_;
 public:
   explicit UnsortedRepFactory(size_t bucket_count = 0, size_t num_locks = 1000)
-    : TransformRepFactory(transform_ = NewNoopTransform(),
+    : TransformRepFactory(NewNoopTransform(),
                           bucket_count,
                           num_locks) { }
-  virtual ~UnsortedRepFactory() { delete transform_; }
 };
 
 // PrefixHashReps bin user keys based on a fixed-size prefix. This optimizes for
@@ -226,6 +231,7 @@ public:
     size_t bucket_count = 0, size_t num_locks = 1000)
     : TransformRepFactory(prefix_extractor, bucket_count, num_locks)
     { }
+
   virtual std::shared_ptr<MemTableRep> CreateMemTableRep(
     MemTableRep::KeyComparator&, Arena*) override;
 };