From: Adam Kupczyk Date: Mon, 3 Dec 2018 15:05:52 +0000 (+0100) Subject: common/KeyValueDB: Get rid of validate parameter. X-Git-Tag: v14.1.0~602^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4eba78af6fbf914526777285ca9265c4203bbf65;p=ceph.git 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 --- diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h index bbea6119fed1..4cfc3891fb45 100644 --- 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 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 2f47cb5358f8..8acea01d6fad 100644 --- 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 17928e9360c1..29dc6aeee543 100644 --- 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 05324916456e..24119de4edc1 100644 --- 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 cfba6dad59dc..5a0570140426 100644 --- 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 a471e4304b73..43855246ffba 100644 --- 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 616ca1b6a25e..e58413f85145 100644 --- 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 3ac7d2dd88e8..b7d71c30fe31 100644 --- 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 f3a7e39abbbe..305d94335758 100644 --- a/src/os/memstore/MemStore.cc +++ b/src/os/memstore/MemStore.cc @@ -583,7 +583,7 @@ public: std::lock_guard lock(o->omap_mutex); return it != o->omap.end(); } - int next(bool validate=true) override { + int next() override { std::lock_guard lock(o->omap_mutex); ++it; return 0; diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h index 532da798824b..fb7d9f82e413 100644 --- a/src/osd/PGLog.h +++ b/src/osd/PGLog.h @@ -1339,7 +1339,7 @@ public: list entries; list 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 e6a8ba1f4750..1a8a63719110 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -7302,7 +7302,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& 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& 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 007bad9a6c69..20bba98a5ff6 100644 --- 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 8366469b5cb4..9972a2f1f22b 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -3010,7 +3010,7 @@ TEST_P(StoreTest, OmapSimple) { { map 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 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 6e1e94e10580..4d5f2b8ea2a7 100644 --- 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")