From e9866245acf09d6d01c912312144cad1397a994b Mon Sep 17 00:00:00 2001
From: Neha Ojha <nojha@redhat.com>
Date: Thu, 28 Apr 2022 21:59:34 +0000
Subject: [PATCH] Revert "bluestore: add config option to allow rocksdb
 iterator bounds to be disabled"

This reverts commit 7d9603058dc9d59c608976d822d59f4738960d16.

Caused a regression https://tracker.ceph.com/issues/55444

Signed-off-by: Neha Ojha <nojha@redhat.com>
---
 src/common/legacy_config_opts.h |  1 -
 src/common/options.cc           |  4 ----
 src/kv/RocksDBStore.cc          | 26 +++++++++++---------------
 src/kv/RocksDBStore.h           | 12 +++++-------
 src/os/bluestore/BlueStore.cc   | 15 ++++++++++++---
 5 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index 3dbd1fe464a83..059113b8cc782 100644
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -768,7 +768,6 @@ OPTION(osd_failsafe_full_ratio, OPT_FLOAT) // what % full makes an OSD "full" (f
 OPTION(osd_fast_shutdown, OPT_BOOL)
 OPTION(osd_fast_shutdown_notify_mon, OPT_BOOL) // tell mon the OSD is shutting down on osd_fast_shutdown
 OPTION(osd_fast_fail_on_connection_refused, OPT_BOOL) // immediately mark OSDs as down once they refuse to accept connections
-OPTION(osd_rocksdb_iterator_bounds_enabled, OPT_BOOL) // whether omap iterator bounds are applied to rocksdb iterator ReadOptions
 
 OPTION(osd_pg_object_context_cache_count, OPT_INT)
 OPTION(osd_tracing, OPT_BOOL) // true if LTTng-UST tracepoints should be enabled
diff --git a/src/common/options.cc b/src/common/options.cc
index 7fda073872332..9a073faa9ed46 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -3798,10 +3798,6 @@ std::vector<Option> get_global_options() {
     .set_flag(Option::FLAG_RUNTIME)
     .set_description("Time in seconds to sleep before next removal transaction when data is on HDD and journal is on SSD"),
 
-    Option("osd_rocksdb_iterator_bounds_enabled", Option::TYPE_BOOL, Option::LEVEL_DEV)
-    .set_default(true)
-    .set_description("Whether omap iterator bounds are applied to rocksdb iterator ReadOptions"),
-
     Option("osd_failsafe_full_ratio", Option::TYPE_FLOAT, Option::LEVEL_ADVANCED)
     .set_default(.97)
     .set_description(""),
diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc
index d7a5ee707047d..dcfa645dc01c5 100644
--- a/src/kv/RocksDBStore.cc
+++ b/src/kv/RocksDBStore.cc
@@ -2250,13 +2250,11 @@ public:
       iterate_upper_bound(make_slice(bounds.upper_bound))
       {
       auto options = rocksdb::ReadOptions();
-      if (db->cct->_conf->osd_rocksdb_iterator_bounds_enabled) {
-        if (bounds.lower_bound) {
-          options.iterate_lower_bound = &iterate_lower_bound;
-        }
-        if (bounds.upper_bound) {
-          options.iterate_upper_bound = &iterate_upper_bound;
-        }
+      if (bounds.lower_bound) {
+        options.iterate_lower_bound = &iterate_lower_bound;
+      }
+      if (bounds.upper_bound) {
+        options.iterate_upper_bound = &iterate_upper_bound;
       }
       dbiter = db->db->NewIterator(options, cf);
   }
@@ -2806,13 +2804,11 @@ public:
   {
     iters.reserve(shards.size());
     auto options = rocksdb::ReadOptions();
-    if (db->cct->_conf->osd_rocksdb_iterator_bounds_enabled) {
-      if (bounds.lower_bound) {
-        options.iterate_lower_bound = &iterate_lower_bound;
-      }
-      if (bounds.upper_bound) {
-        options.iterate_upper_bound = &iterate_upper_bound;
-      }
+    if (bounds.lower_bound) {
+      options.iterate_lower_bound = &iterate_lower_bound;
+    }
+    if (bounds.upper_bound) {
+      options.iterate_upper_bound = &iterate_upper_bound;
     }
     for (auto& s : shards) {
       iters.push_back(db->db->NewIterator(options, s));
@@ -2993,7 +2989,7 @@ KeyValueDB::Iterator RocksDBStore::get_iterator(const std::string& prefix, Itera
     rocksdb::ColumnFamilyHandle* cf = nullptr;
     if (cf_it->second.handles.size() == 1) {
       cf = cf_it->second.handles[0];
-    } else if (cct->_conf->osd_rocksdb_iterator_bounds_enabled) {
+    } else {
       cf = get_cf_handle(prefix, bounds);
     }
     if (cf) {
diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h
index a64e69b1d1d6c..37833bf74a4d3 100644
--- a/src/kv/RocksDBStore.h
+++ b/src/kv/RocksDBStore.h
@@ -369,13 +369,11 @@ public:
         rocksdb::ReadOptions options = rocksdb::ReadOptions();
         if (opts & ITERATOR_NOCACHE)
           options.fill_cache=false;
-        if (db->cct->_conf->osd_rocksdb_iterator_bounds_enabled) {
-          if (bounds.lower_bound) {
-            options.iterate_lower_bound = &iterate_lower_bound;
-          }
-          if (bounds.upper_bound) {
-            options.iterate_upper_bound = &iterate_upper_bound;
-          }
+        if (bounds.lower_bound) {
+          options.iterate_lower_bound = &iterate_lower_bound;
+        }
+        if (bounds.upper_bound) {
+          options.iterate_upper_bound = &iterate_upper_bound;
         }
         dbiter = db->db->NewIterator(options, cf);
     }
diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index b9ee17fbfe830..43a206e552512 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -11004,7 +11004,10 @@ int BlueStore::_onode_omap_get(
     string head, tail;
     o->get_omap_header(&head);
     o->get_omap_tail(&tail);
-    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, KeyValueDB::IteratorBounds{head, tail});
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = head;
+    bounds.upper_bound = tail;
+    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, std::move(bounds));
     it->lower_bound(head);
     while (it->valid()) {
       if (it->key() == head) {
@@ -11089,7 +11092,10 @@ int BlueStore::omap_get_keys(
     string head, tail;
     o->get_omap_key(string(), &head);
     o->get_omap_tail(&tail);
-    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, KeyValueDB::IteratorBounds{head, tail});
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = head;
+    bounds.upper_bound = tail;
+    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, std::move(bounds));
     it->lower_bound(head);
     while (it->valid()) {
       if (it->key() >= tail) {
@@ -15551,7 +15557,10 @@ int BlueStore::_clone(TransContext *txc,
     string head, tail;
     oldo->get_omap_header(&head);
     oldo->get_omap_tail(&tail);
-    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, KeyValueDB::IteratorBounds{head, tail});
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = head;
+    bounds.upper_bound = tail;
+    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, std::move(bounds));
     it->lower_bound(head);
     while (it->valid()) {
       if (it->key() >= tail) {
-- 
2.39.5