From: Casey Bodley <cbodley@redhat.com>
Date: Tue, 6 Aug 2019 20:48:52 +0000 (-0400)
Subject: kv/rocksdb: support rmrange unconditionally
X-Git-Tag: v15.1.0~1792^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=49672ad5c459b98d8dec94db54c2cb5b7da5ce8f;p=ceph-ci.git

kv/rocksdb: support rmrange unconditionally

removes the config options rocksdb_enable_rmrange and
rocksdb_max_items_rmrange that avoid calls to DeleteRange()

Signed-off-by: Casey Bodley <cbodley@redhat.com>
---

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index df13b12c80f..bb7541fd108 100644
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -828,7 +828,6 @@ OPTION(rocksdb_perf, OPT_BOOL) // Enabling this will have 5-10% impact on perfor
 OPTION(rocksdb_collect_compaction_stats, OPT_BOOL) //For rocksdb, this behavior will be an overhead of 5%~10%, collected only rocksdb_perf is enabled.
 OPTION(rocksdb_collect_extended_stats, OPT_BOOL) //For rocksdb, this behavior will be an overhead of 5%~10%, collected only rocksdb_perf is enabled.
 OPTION(rocksdb_collect_memory_stats, OPT_BOOL) //For rocksdb, this behavior will be an overhead of 5%~10%, collected only rocksdb_perf is enabled.
-OPTION(rocksdb_enable_rmrange, OPT_BOOL) // see https://github.com/facebook/rocksdb/blob/master/include/rocksdb/db.h#L253
 
 // rocksdb options that will be used for omap(if omap_backend is rocksdb)
 OPTION(filestore_rocksdb_options, OPT_STR)
diff --git a/src/common/options.cc b/src/common/options.cc
index c9160fc6c27..554de3d98e1 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -3947,15 +3947,6 @@ std::vector<Option> get_global_options() {
     .set_default(false)
     .set_description(""),
 
-    Option("rocksdb_enable_rmrange", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
-    .set_default(true)
-    .set_description("Refer to github.com/facebook/rocksdb/wiki/DeleteRange-Implementation"),
-
-    Option("rocksdb_max_items_rmrange", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
-    .set_default(1024)
-    .set_description("Delete Range will be called if number of keys exceeded, must enable rocksdb_enable_rmrange first")
-    .add_see_also("rocksdb_enable_rmrange"),
-
     Option("rocksdb_bloom_bits_per_key", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
     .set_default(20)
     .set_description("Number of bits per key to use for RocksDB's bloom filters.")
diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc
index 4756697f270..bf60df372d9 100644
--- a/src/kv/RocksDBStore.cc
+++ b/src/kv/RocksDBStore.cc
@@ -944,70 +944,14 @@ void RocksDBStore::RocksDBTransactionImpl::rmkeys_by_prefix(const string &prefix
 {
   auto cf = db->get_cf_handle(prefix);
   if (cf) {
-    if (db->enable_rmrange) {
-      string endprefix("\xff\xff\xff\xff");  // FIXME: this is cheating...
-      if (db->max_items_rmrange) {
-        uint64_t cnt = db->max_items_rmrange;
-        bat.SetSavePoint();
-        auto it = db->get_iterator(prefix);
-        for (it->seek_to_first();
-        it->valid();
-        it->next()) {
-          if (!cnt) {
-            bat.RollbackToSavePoint();
-            bat.DeleteRange(cf, string(), endprefix);
-            return;
-          }
-          bat.Delete(cf, rocksdb::Slice(it->key()));
-          --cnt;
-        }
-        bat.PopSavePoint();
-      } else {
-        bat.DeleteRange(cf, string(), endprefix);
-      }
-    } else {
-      auto it = db->get_iterator(prefix);
-      for (it->seek_to_first();
-	   it->valid();
-	   it->next()) {
-	bat.Delete(cf, rocksdb::Slice(it->key()));
-      }
-    }
+    string endprefix("\xff\xff\xff\xff");  // FIXME: this is cheating...
+    bat.DeleteRange(cf, string(), endprefix);
   } else {
-    if (db->enable_rmrange) {
-      string endprefix = prefix;
-      endprefix.push_back('\x01');
-      if (db->max_items_rmrange) {
-        uint64_t cnt = db->max_items_rmrange;
-        bat.SetSavePoint();
-        auto it = db->get_iterator(prefix);
-        for (it->seek_to_first();
-             it->valid();
-             it->next()) {
-          if (!cnt) {
-            bat.RollbackToSavePoint();
-            bat.DeleteRange(db->default_cf,
-                combine_strings(prefix, string()),
-                combine_strings(endprefix, string()));
-            return;
-          }
-          bat.Delete(db->default_cf, combine_strings(prefix, it->key()));
-          --cnt;
-        }
-        bat.PopSavePoint();
-      } else {
-        bat.DeleteRange(db->default_cf,
-            combine_strings(prefix, string()),
-            combine_strings(endprefix, string()));
-      }
-    } else {
-      auto it = db->get_iterator(prefix);
-      for (it->seek_to_first();
-	   it->valid();
-	   it->next()) {
-	bat.Delete(db->default_cf, combine_strings(prefix, it->key()));
-      }
-    }
+    string endprefix = prefix;
+    endprefix.push_back('\x01');
+    bat.DeleteRange(db->default_cf,
+      combine_strings(prefix, string()),
+      combine_strings(endprefix, string()));
   }
 }
 
@@ -1017,83 +961,12 @@ void RocksDBStore::RocksDBTransactionImpl::rm_range_keys(const string &prefix,
 {
   auto cf = db->get_cf_handle(prefix);
   if (cf) {
-    if (db->enable_rmrange) {
-      if (db->max_items_rmrange) {
-        uint64_t cnt = db->max_items_rmrange;
-        auto it = db->get_iterator(prefix);
-        bat.SetSavePoint();
-        it->lower_bound(start);
-        while (it->valid()) {
-          if (it->key() >= end) {
-            break;
-          }
-          if (!cnt) {
-            bat.RollbackToSavePoint();
-            bat.DeleteRange(cf, rocksdb::Slice(start), rocksdb::Slice(end));
-            return;
-          }
-          bat.Delete(cf, rocksdb::Slice(it->key()));
-          it->next();
-          --cnt;
-        }
-        bat.PopSavePoint();
-      } else {
-        bat.DeleteRange(cf, rocksdb::Slice(start), rocksdb::Slice(end));
-      }
-    } else {
-      auto it = db->get_iterator(prefix);
-      it->lower_bound(start);
-      while (it->valid()) {
-	if (it->key() >= end) {
-	  break;
-	}
-	bat.Delete(cf, rocksdb::Slice(it->key()));
-	it->next();
-      }
-    }
+    bat.DeleteRange(cf, rocksdb::Slice(start), rocksdb::Slice(end));
   } else {
-    if (db->enable_rmrange) {
-      if (db->max_items_rmrange) {
-        uint64_t cnt = db->max_items_rmrange;
-        auto it = db->get_iterator(prefix);
-        bat.SetSavePoint();
-        it->lower_bound(start);
-        while (it->valid()) {
-          if (it->key() >= end) {
-            break;
-          }
-          if (!cnt) {
-            bat.RollbackToSavePoint();
-            bat.DeleteRange(
-                db->default_cf,
-                rocksdb::Slice(combine_strings(prefix, start)),
-                rocksdb::Slice(combine_strings(prefix, end)));
-            return;
-          }
-          bat.Delete(db->default_cf,
-          combine_strings(prefix, it->key()));
-          it->next();
-          --cnt;
-        }
-        bat.PopSavePoint();
-      } else {
-        bat.DeleteRange(
-            db->default_cf,
-            rocksdb::Slice(combine_strings(prefix, start)),
-            rocksdb::Slice(combine_strings(prefix, end)));
-      }
-    } else {
-      auto it = db->get_iterator(prefix);
-      it->lower_bound(start);
-      while (it->valid()) {
-	if (it->key() >= end) {
-	  break;
-	}
-	bat.Delete(db->default_cf,
-		   combine_strings(prefix, it->key()));
-	it->next();
-      }
-    }
+    bat.DeleteRange(
+        db->default_cf,
+        rocksdb::Slice(combine_strings(prefix, start)),
+        rocksdb::Slice(combine_strings(prefix, end)));
   }
 }
 
diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h
index ea7c77d3c76..9557ef1d258 100644
--- a/src/kv/RocksDBStore.h
+++ b/src/kv/RocksDBStore.h
@@ -119,8 +119,6 @@ public:
   /// compact the underlying rocksdb store
   bool compact_on_mount;
   bool disableWAL;
-  bool enable_rmrange;
-  const uint64_t max_items_rmrange;
   void compact() override;
 
   void compact_async() override {
@@ -159,9 +157,7 @@ public:
     compact_queue_stop(false),
     compact_thread(this),
     compact_on_mount(false),
-    disableWAL(false),
-    enable_rmrange(cct->_conf->rocksdb_enable_rmrange),
-    max_items_rmrange(cct->_conf.get_val<uint64_t>("rocksdb_max_items_rmrange"))
+    disableWAL(false)
   {}
 
   ~RocksDBStore() override;