common/KeyValueDB: Get rid of validate parameter.

  Operations next() and prev() on iterator may force check is current iterator is in valid state.
  However, all usages of next() perform valid() earlier, and most skip return value altogether.
  Getting rid of this allows to unify KeyValueDB::IteratorImpl and KeyValueDB::WholeSpaceIteratorImpl under some basic iterator interface.

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>

diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h
index bbea6119fed1c04f52deabbfccba03dfaf6bfcf6..4cfc3891fb459d3ea2065be4a5af1287eeea679c 100644 (file)
--- a/src/kv/KeyValueDB.h
+++ b/src/kv/KeyValueDB.h
@@ -208,7 +208,7 @@ public:
     virtual int upper_bound(const std::string &after) = 0;
     virtual int lower_bound(const std::string &to) = 0;
     virtual bool valid() = 0;
-    virtual int next(bool validate=true) = 0;
+    virtual int next() = 0;
     virtual std::string key() = 0;
     virtual bufferlist value() = 0;
     virtual int status() = 0;
@@ -219,7 +219,7 @@ public:
   public:
     virtual ~IteratorImpl() {}
     virtual int seek_to_last() = 0;
-    virtual int prev(bool validate=true) = 0;
+    virtual int prev() = 0;
     virtual std::pair<std::string, std::string> raw_key() = 0;
     virtual bufferptr value_as_ptr() {
       bufferlist bl = value();
@@ -299,26 +299,11 @@ private:
        return false;
       return generic_iter->raw_key_is_prefixed(prefix);
     }
-    // Note that next() and prev() shouldn't validate iters,
-    // it's responsibility of caller to ensure they're valid.
-    int next(bool validate=true) override {
-      if (validate) {
-        if (valid())
-          return generic_iter->next();
-        return status();
-      } else {
-        return generic_iter->next();  
-      }      
+    int next() override {
+      return generic_iter->next();
     }
-    
-    int prev(bool validate=true) override {
-      if (validate) {
-        if (valid())
-          return generic_iter->prev();
-        return status();
-      } else {
-        return generic_iter->prev();  
-      }      
+    int prev() override {
+      return generic_iter->prev();
     }
     std::string key() override {
       return generic_iter->key();

diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc
index 2f47cb5358f80def84e3b5dccc6d7dd82b32df8e..8acea01d6fad244fbb39bae6007c6a48c9bf9a57 100644 (file)
--- a/src/kv/RocksDBStore.cc
+++ b/src/kv/RocksDBStore.cc
@@ -1504,13 +1504,13 @@ public:
     dbiter->Seek(slice_bound);
     return dbiter->status().ok() ? 0 : -1;
   }
-  int next(bool validate=true) override {
+  int next() override {
     if (valid()) {
       dbiter->Next();
     }
     return dbiter->status().ok() ? 0 : -1;
   }
-  int prev(bool validate=true) override {
+  int prev() override {
     if (valid()) {
       dbiter->Prev();
     }

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 17928e9360c141a012ff871b6a6375fc8c13f558..29dc6aeee543c03e85e70c6d5ac3ea8aa982f3b4 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -3780,7 +3780,7 @@ bool BlueStore::OmapIteratorImpl::valid()
   return r;
 }
 
-int BlueStore::OmapIteratorImpl::next(bool validate)
+int BlueStore::OmapIteratorImpl::next()
 {
   RWLock::RLocker l(c->lock);
   if (o->onode.has_omap()) {

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 05324916456eac0c6ca92f4c6267b1834712e4e6..24119de4edc17977067c7ddc2d74e28b198a0184 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -1434,7 +1434,7 @@ public:
     int upper_bound(const string &after) override;
     int lower_bound(const string &to) override;
     bool valid() override;
-    int next(bool validate=true) override;
+    int next() override;
     string key() override;
     bufferlist value() override;
     int status() override {

diff --git a/src/os/filestore/DBObjectMap.cc b/src/os/filestore/DBObjectMap.cc
index cfba6dad59dc68db7849e0d33d90811e131ce13f..5a057014042679df4ebfbf691c50dd9bcf2159da 100644 (file)
--- a/src/os/filestore/DBObjectMap.cc
+++ b/src/os/filestore/DBObjectMap.cc
@@ -402,7 +402,7 @@ bool DBObjectMap::DBObjectMapIteratorImpl::valid_parent()
   return false;
 }
 
-int DBObjectMap::DBObjectMapIteratorImpl::next(bool validate)
+int DBObjectMap::DBObjectMapIteratorImpl::next()
 {
   ceph_assert(cur_iter->valid());
   ceph_assert(valid());

diff --git a/src/os/filestore/DBObjectMap.h b/src/os/filestore/DBObjectMap.h
index a471e4304b734891edeb132279228c62c9b6d965..43855246ffba8e6170c26543c437bd751f4e3d60 100644 (file)
--- a/src/os/filestore/DBObjectMap.h
+++ b/src/os/filestore/DBObjectMap.h
@@ -387,7 +387,7 @@ private:
     int upper_bound(const string &after) override { return 0; }
     int lower_bound(const string &to) override { return 0; }
     bool valid() override { return false; }
-    int next(bool validate=true) override { ceph_abort(); return 0; }
+    int next() override { ceph_abort(); return 0; }
     string key() override { ceph_abort(); return ""; }
     bufferlist value() override { ceph_abort(); return bufferlist(); }
     int status() override { return 0; }
@@ -425,7 +425,7 @@ private:
     int upper_bound(const string &after) override;
     int lower_bound(const string &to) override;
     bool valid() override;
-    int next(bool validate=true) override;
+    int next() override;
     string key() override;
     bufferlist value() override;
     int status() override;

diff --git a/src/os/kstore/KStore.cc b/src/os/kstore/KStore.cc
index 616ca1b6a25eed8c290efb1bfd8536aa9e5ac27d..e58413f851450ff56b4893abec2d8fe40d89348a 100644 (file)
--- a/src/os/kstore/KStore.cc
+++ b/src/os/kstore/KStore.cc
@@ -1636,7 +1636,7 @@ bool KStore::OmapIteratorImpl::valid()
   }
 }
 
-int KStore::OmapIteratorImpl::next(bool validate)
+int KStore::OmapIteratorImpl::next()
 {
   RWLock::RLocker l(c->lock);
   if (o->onode.omap_head) {

diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h
index 3ac7d2dd88e88196a3aa5ba0e81ac3d7ca867f84..b7d71c30fe311482bc6dcaa1fd90a6c89e61911e 100644 (file)
--- a/src/os/kstore/KStore.h
+++ b/src/os/kstore/KStore.h
@@ -175,7 +175,7 @@ public:
     int upper_bound(const string &after) override;
     int lower_bound(const string &to) override;
     bool valid() override;
-    int next(bool validate=true) override;
+    int next() override;
     string key() override;
     bufferlist value() override;
     int status() override {

diff --git a/src/os/memstore/MemStore.cc b/src/os/memstore/MemStore.cc
index f3a7e39abbbe890484e9d7024dd3b32ff026b798..305d943357584eb4884aba041249ec544c02dd92 100644 (file)
--- a/src/os/memstore/MemStore.cc
+++ b/src/os/memstore/MemStore.cc
@@ -583,7 +583,7 @@ public:
     std::lock_guard<std::mutex> lock(o->omap_mutex);
     return it != o->omap.end();
   }
-  int next(bool validate=true) override {
+  int next() override {
     std::lock_guard<std::mutex> lock(o->omap_mutex);
     ++it;
     return 0;

diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h
index 532da798824b90eb416d2bbe81c466880ffb2ab8..fb7d9f82e413004f8d5d339724fdd7e380c3cc17 100644 (file)
--- a/src/osd/PGLog.h
+++ b/src/osd/PGLog.h
@@ -1339,7 +1339,7 @@ public:
     list<pg_log_entry_t> entries;
     list<pg_log_dup_t> dups;
     if (p) {
-      for (p->seek_to_first(); p->valid() ; p->next(false)) {
+      for (p->seek_to_first(); p->valid() ; p->next()) {
        // non-log pgmeta_oid keys are prefixed with _; skip those
        if (p->key()[0] == '_')
          continue;

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index e6a8ba1f4750e708a16b50c64969039104d5e0eb..1a8a637191102581ba6ebc020ba0a3e346b2ef65 100644 (file)
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -7302,7 +7302,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
            );
          ceph_assert(iter);
          iter->upper_bound(start_after);
-         for (num = 0; iter->valid(); ++num, iter->next(false)) {
+         for (num = 0; iter->valid(); ++num, iter->next()) {
            if (num >= max_return ||
                bl.length() >= cct->_conf->osd_max_omap_bytes_per_request) {
              truncated = true;
@@ -7356,7 +7356,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
          for (num = 0;
               iter->valid() &&
                 iter->key().substr(0, filter_prefix.size()) == filter_prefix;
-              ++num, iter->next(false)) {
+              ++num, iter->next()) {
            dout(20) << "Found key " << iter->key() << dendl;
            if (num >= max_return ||
                bl.length() >= cct->_conf->osd_max_omap_bytes_per_request) {
@@ -8762,7 +8762,7 @@ int PrimaryLogPG::do_copy_get(OpContext *ctx, bufferlist::const_iterator& bp,
        osd->store->get_omap_iterator(ch, ghobject_t(oi.soid));
       ceph_assert(iter);
       iter->upper_bound(cursor.omap_offset);
-      for (; iter->valid(); iter->next(false)) {
+      for (; iter->valid(); iter->next()) {
        ++omap_keys;
        encode(iter->key(), omap_data);
        encode(iter->value(), omap_data);

diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc
index 007bad9a6c690b79f842f0c77fe391d9f029617e..20bba98a5ff6b6e221d0c9c6faa9936bb3f4ae8d 100644 (file)
--- a/src/osd/ReplicatedBackend.cc
+++ b/src/osd/ReplicatedBackend.cc
@@ -1877,7 +1877,7 @@ int ReplicatedBackend::build_push_op(const ObjectRecoveryInfo &recovery_info,
     ceph_assert(iter);
     for (iter->lower_bound(progress.omap_recovered_to);
         iter->valid();
-        iter->next(false)) {
+        iter->next()) {
       if (!out_op->omap_entries.empty() &&
          ((cct->_conf->osd_recovery_max_omap_entries_per_chunk > 0 &&
            out_op->omap_entries.size() >= cct->_conf->osd_recovery_max_omap_entries_per_chunk) ||

diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
index 8366469b5cb4ccc1e9dda833fabf16862ef598d9..9972a2f1f22b89654ef6d3a38f4af1f1ebd08966 100644 (file)
--- a/src/test/objectstore/store_test.cc
+++ b/src/test/objectstore/store_test.cc
@@ -3010,7 +3010,7 @@ TEST_P(StoreTest, OmapSimple) {
   {
     map<string,bufferlist> r;
     ObjectMap::ObjectMapIterator iter = store->get_omap_iterator(ch, hoid);
-    for (iter->seek_to_first(); iter->valid(); iter->next(false)) {
+    for (iter->seek_to_first(); iter->valid(); iter->next()) {
       r[iter->key()] = iter->value();
     }
     cout << "r: " << r << std::endl;
@@ -3020,7 +3020,7 @@ TEST_P(StoreTest, OmapSimple) {
   {
     map<string,bufferlist> r;
     ObjectMap::ObjectMapIterator iter = store->get_omap_iterator(ch, hoid);
-    for (iter->lower_bound(string()); iter->valid(); iter->next(false)) {
+    for (iter->lower_bound(string()); iter->valid(); iter->next()) {
       r[iter->key()] = iter->value();
     }
     cout << "r: " << r << std::endl;

diff --git a/src/tools/ceph_objectstore_tool.cc b/src/tools/ceph_objectstore_tool.cc
index 6e1e94e1058089a80919868b840079b83956f229..4d5f2b8ea2a727926160575f7eafd6e3304871f8 100644 (file)
--- a/src/tools/ceph_objectstore_tool.cc
+++ b/src/tools/ceph_objectstore_tool.cc
@@ -534,7 +534,7 @@ int do_trim_pg_log(ObjectStore *store, const coll_t &coll,
     ObjectMap::ObjectMapIterator p = store->get_omap_iterator(ch, oid);
     if (!p)
       break;
-    for (p->seek_to_first(); p->valid(); p->next(false)) {
+    for (p->seek_to_first(); p->valid(); p->next()) {
       if (p->key()[0] == '_')
        continue;
       if (p->key() == "can_rollback_to")