Unbreak MemTableRep API change

Summary:
The MemTableRep API was broken by this commit: 813719e952
This patch reverts the changes and instead adds InsertKey (and etc.) overloads to extend the MemTableRep API without breaking the existing classes that inherit from it.
Closes https://github.com/facebook/rocksdb/pull/3513

Differential Revision: D7004134

Pulled By: maysamyabandeh

fbshipit-source-id: e568d91fe1e17dd76c0c1f6c7dd51a18633b1c4f

db/db_memtable_test.cc

@@ -28,17 +28,14 @@ class MockMemTableRep : public MemTableRep {
     return rep_->Allocate(len, buf);
   }
 
-  virtual bool Insert(KeyHandle handle) override {
+  virtual void Insert(KeyHandle handle) override { rep_->Insert(handle); }
-    return rep_->Insert(handle);
-  }
 
-  virtual bool InsertWithHint(KeyHandle handle, void** hint) override {
+  virtual void InsertWithHint(KeyHandle handle, void** hint) override {
     num_insert_with_hint_++;
     EXPECT_NE(nullptr, hint);
     last_hint_in_ = *hint;
-    bool res = rep_->InsertWithHint(handle, hint);
+    rep_->InsertWithHint(handle, hint);
     last_hint_out_ = *hint;
-    return res;
   }
 
   virtual bool Contains(const char* key) const override {

db/db_test_util.h

@@ -136,9 +136,9 @@ class SpecialMemTableRep : public MemTableRep {
 
   // Insert key into the list.
   // REQUIRES: nothing that compares equal to key is currently in the list.
-  virtual bool Insert(KeyHandle handle) override {
+  virtual void Insert(KeyHandle handle) override {
     num_entries_++;
-    return memtable_->Insert(handle);
+    memtable_->Insert(handle);
   }
 
   // Returns true iff an entry that compares equal to key is in the list.

db/memtable.cc

@@ -236,7 +236,7 @@ int MemTable::KeyComparator::operator()(const char* prefix_len_key,
   return comparator.CompareKeySeq(a, key);
 }
 
-bool MemTableRep::InsertConcurrently(KeyHandle handle) {
+void MemTableRep::InsertConcurrently(KeyHandle handle) {
 #ifndef ROCKSDB_LITE
   throw std::runtime_error("concurrent insert not supported");
 #else
@@ -478,12 +478,12 @@ bool MemTable::Add(SequenceNumber s, ValueType type,
     if (insert_with_hint_prefix_extractor_ != nullptr &&
         insert_with_hint_prefix_extractor_->InDomain(key_slice)) {
       Slice prefix = insert_with_hint_prefix_extractor_->Transform(key_slice);
-      bool res = table->InsertWithHint(handle, &insert_hints_[prefix]);
+      bool res = table->InsertKeyWithHint(handle, &insert_hints_[prefix]);
       if (!res) {
         return res;
       }
     } else {
-      bool res = table->Insert(handle);
+      bool res = table->InsertKey(handle);
       if (!res) {
         return res;
       }
@@ -519,7 +519,7 @@ bool MemTable::Add(SequenceNumber s, ValueType type,
     assert(post_process_info == nullptr);
     UpdateFlushState();
   } else {
-    bool res = table->InsertConcurrently(handle);
+    bool res = table->InsertKeyConcurrently(handle);
     if (!res) {
       return res;
     }

include/rocksdb/memtablerep.h

@@ -81,10 +81,15 @@ class MemTableRep {
   // single buffer and pass that in as the parameter to Insert).
   // REQUIRES: nothing that compares equal to key is currently in the
   // collection, and no concurrent modifications to the table in progress
-  //
+  virtual void Insert(KeyHandle handle) = 0;
+
+  // Same as ::Insert
   // Returns false if MemTableRepFactory::CanHandleDuplicatedKey() is true and
   // the <key, seq> already exists.
-  virtual bool Insert(KeyHandle handle) = 0;
+  virtual bool InsertKey(KeyHandle handle) {
+    Insert(handle);
+    return true;
+  }
 
   // Same as Insert(), but in additional pass a hint to insert location for
   // the key. If hint points to nullptr, a new hint will be populated.
@@ -92,12 +97,17 @@ class MemTableRep {
   //
   // Currently only skip-list based memtable implement the interface. Other
   // implementations will fallback to Insert() by default.
-  //
+  virtual void InsertWithHint(KeyHandle handle, void** hint) {
+    // Ignore the hint by default.
+    Insert(handle);
+  }
+
+  // Same as ::InsertWithHint
   // Returns false if MemTableRepFactory::CanHandleDuplicatedKey() is true and
   // the <key, seq> already exists.
-  virtual bool InsertWithHint(KeyHandle handle, void** hint) {
+  virtual bool InsertKeyWithHint(KeyHandle handle, void** hint) {
-    // Ignore the hint by default.
+    InsertWithHint(handle, hint);
-    return Insert(handle);
+    return true;
   }
 
   // Like Insert(handle), but may be called concurrent with other calls
@@ -105,7 +115,15 @@ class MemTableRep {
   //
   // Returns false if MemTableRepFactory::CanHandleDuplicatedKey() is true and
   // the <key, seq> already exists.
-  virtual bool InsertConcurrently(KeyHandle handle);
+  virtual void InsertConcurrently(KeyHandle handle);
+
+  // Same as ::InsertConcurrently
+  // Returns false if MemTableRepFactory::CanHandleDuplicatedKey() is true and
+  // the <key, seq> already exists.
+  virtual bool InsertKeyConcurrently(KeyHandle handle) {
+    InsertConcurrently(handle);
+    return true;
+  }
 
   // Returns true iff an entry that compares equal to key is in the collection.
   virtual bool Contains(const char* key) const = 0;

memtable/hash_cuckoo_rep.cc

@@ -97,7 +97,7 @@ class HashCuckooRep : public MemTableRep {
   // Insert the specified key (internal_key) into the mem-table.  Assertion
   // fails if
   // the current mem-table already contains the specified key.
-  virtual bool Insert(KeyHandle handle) override;
+  virtual void Insert(KeyHandle handle) override;
 
   // This function returns bucket_count_ * approximate_entry_size_ when any
   // of the followings happen to disallow further write operations:
@@ -319,7 +319,7 @@ void HashCuckooRep::Get(const LookupKey& key, void* callback_args,
   }
 }
 
-bool HashCuckooRep::Insert(KeyHandle handle) {
+void HashCuckooRep::Insert(KeyHandle handle) {
   static const float kMaxFullness = 0.90f;
 
   auto* key = static_cast<char*>(handle);
@@ -340,7 +340,7 @@ bool HashCuckooRep::Insert(KeyHandle handle) {
       is_nearly_full_ = true;
     }
     backup_table_->Insert(key);
-    return true;
+    return;
   }
   // when reaching this point, means the insert can be done successfully.
   occupied_count_++;
@@ -349,7 +349,7 @@ bool HashCuckooRep::Insert(KeyHandle handle) {
   }
 
   // perform kickout process if the length of cuckoo path > 1.
-  if (cuckoo_path_length == 0) return true;
+  if (cuckoo_path_length == 0) return;
 
   // the cuckoo path stores the kickout path in reverse order.
   // so the kickout or displacement is actually performed
@@ -366,7 +366,6 @@ bool HashCuckooRep::Insert(KeyHandle handle) {
   }
   int insert_key_bid = cuckoo_path_[cuckoo_path_length - 1];
   cuckoo_array_[insert_key_bid].store(key, std::memory_order_release);
-  return true;
 }
 
 bool HashCuckooRep::Contains(const char* internal_key) const {

memtable/hash_linklist_rep.cc

@@ -170,7 +170,7 @@ class HashLinkListRep : public MemTableRep {
 
   virtual KeyHandle Allocate(const size_t len, char** buf) override;
 
-  virtual bool Insert(KeyHandle handle) override;
+  virtual void Insert(KeyHandle handle) override;
 
   virtual bool Contains(const char* key) const override;
 
@@ -571,7 +571,7 @@ Node* HashLinkListRep::GetLinkListFirstNode(Pointer* first_next_pointer) const {
   return nullptr;
 }
 
-bool HashLinkListRep::Insert(KeyHandle handle) {
+void HashLinkListRep::Insert(KeyHandle handle) {
   Node* x = static_cast<Node*>(handle);
   assert(!Contains(x->key));
   Slice internal_key = GetLengthPrefixedSlice(x->key);
@@ -586,7 +586,7 @@ bool HashLinkListRep::Insert(KeyHandle handle) {
     // we publish a pointer to "x" in prev[i].
     x->NoBarrier_SetNext(nullptr);
     bucket.store(x, std::memory_order_release);
-    return true;
+    return;
   }
 
   BucketHeader* header = nullptr;
@@ -613,7 +613,7 @@ bool HashLinkListRep::Insert(KeyHandle handle) {
       // incremental.
       skip_list_bucket_header->Counting_header.IncNumEntries();
       skip_list_bucket_header->skip_list.Insert(x->key);
-      return true;
+      return;
     }
   }
 
@@ -691,7 +691,6 @@ bool HashLinkListRep::Insert(KeyHandle handle) {
       header->next.store(static_cast<void*>(x), std::memory_order_release);
     }
   }
-  return true;
 }
 
 bool HashLinkListRep::Contains(const char* key) const {

memtable/hash_skiplist_rep.cc

@@ -28,7 +28,7 @@ class HashSkipListRep : public MemTableRep {
                   size_t bucket_size, int32_t skiplist_height,
                   int32_t skiplist_branching_factor);
 
-  virtual bool Insert(KeyHandle handle) override;
+  virtual void Insert(KeyHandle handle) override;
 
   virtual bool Contains(const char* key) const override;
 
@@ -267,13 +267,12 @@ HashSkipListRep::Bucket* HashSkipListRep::GetInitializedBucket(
   return bucket;
 }
 
-bool HashSkipListRep::Insert(KeyHandle handle) {
+void HashSkipListRep::Insert(KeyHandle handle) {
   auto* key = static_cast<char*>(handle);
   assert(!Contains(key));
   auto transformed = transform_->Transform(UserKey(key));
   auto bucket = GetInitializedBucket(transformed);
   bucket->Insert(key);
-  return true;
 }
 
 bool HashSkipListRep::Contains(const char* key) const {

memtable/skiplistrep.cc

@@ -34,15 +34,27 @@ public:
 
   // Insert key into the list.
   // REQUIRES: nothing that compares equal to key is currently in the list.
-  virtual bool Insert(KeyHandle handle) override {
+  virtual void Insert(KeyHandle handle) override {
+    skip_list_.Insert(static_cast<char*>(handle));
+  }
+
+  virtual bool InsertKey(KeyHandle handle) override {
     return skip_list_.Insert(static_cast<char*>(handle));
   }
 
-  virtual bool InsertWithHint(KeyHandle handle, void** hint) override {
+  virtual void InsertWithHint(KeyHandle handle, void** hint) override {
+    skip_list_.InsertWithHint(static_cast<char*>(handle), hint);
+  }
+
+  virtual bool InsertKeyWithHint(KeyHandle handle, void** hint) override {
     return skip_list_.InsertWithHint(static_cast<char*>(handle), hint);
   }
 
-  virtual bool InsertConcurrently(KeyHandle handle) override {
+  virtual void InsertConcurrently(KeyHandle handle) override {
+    skip_list_.InsertConcurrently(static_cast<char*>(handle));
+  }
+
+  virtual bool InsertKeyConcurrently(KeyHandle handle) override {
     return skip_list_.InsertConcurrently(static_cast<char*>(handle));
   }
 

memtable/vectorrep.cc

@@ -31,7 +31,7 @@ class VectorRep : public MemTableRep {
   // single buffer and pass that in as the parameter to Insert)
   // REQUIRES: nothing that compares equal to key is currently in the
   // collection.
-  virtual bool Insert(KeyHandle handle) override;
+  virtual void Insert(KeyHandle handle) override;
 
   // Returns true iff an entry that compares equal to key is in the collection.
   virtual bool Contains(const char* key) const override;
@@ -108,12 +108,11 @@ class VectorRep : public MemTableRep {
   const KeyComparator& compare_;
 };
 
-bool VectorRep::Insert(KeyHandle handle) {
+void VectorRep::Insert(KeyHandle handle) {
   auto* key = static_cast<char*>(handle);
   WriteLock l(&rwlock_);
   assert(!immutable_);
   bucket_->push_back(key);
-  return true;
 }
 
 // Returns true iff an entry that compares equal to key is in the collection.