From c34ab169b19e346a7fdcee22d4630a500a083216 Mon Sep 17 00:00:00 2001 From: chunmei-liu Date: Fri, 18 Dec 2020 20:48:42 -0800 Subject: [PATCH] crimson/seastore: fix comments for omap tree Signed-off-by: chunmei-liu --- src/crimson/os/seastore/omap_manager.h | 89 +++++----- .../omap_manager/btree/btree_omap_manager.cc | 57 ++++--- .../omap_manager/btree/btree_omap_manager.h | 16 +- .../omap_manager/btree/omap_btree_node.h | 5 +- .../btree/omap_btree_node_impl.cc | 152 +++++++++--------- .../omap_manager/btree/omap_btree_node_impl.h | 2 + .../seastore/omap_manager/btree/omap_types.h | 58 +++---- .../btree/string_kv_node_layout.h | 46 +++--- .../os/seastore/transaction_manager.cc | 6 +- src/crimson/os/seastore/transaction_manager.h | 4 +- .../crimson/seastore/test_omap_manager.cc | 12 +- 11 files changed, 226 insertions(+), 221 deletions(-) diff --git a/src/crimson/os/seastore/omap_manager.h b/src/crimson/os/seastore/omap_manager.h index 6725bc031ed..b26b4eecb3b 100644 --- a/src/crimson/os/seastore/omap_manager.h +++ b/src/crimson/os/seastore/omap_manager.h @@ -35,12 +35,12 @@ struct omap_root_t { struct list_keys_result_t { std::vector keys; - std::string next; + std::optional next; // if return std::nullopt, means list all keys in omap tree. }; struct list_kvs_result_t { std::vector> kvs; - std::string next; + std::optional next; // if return std::nullopt, means list all kv mapping in omap tree. }; constexpr size_t MAX_SIZE = std::numeric_limits::max(); std::ostream &operator<<(std::ostream &out, const std::list &rhs); @@ -52,88 +52,95 @@ class OMapManager { * until these functions future resolved. */ public: - /* allocate omap tree root node + /** + * allocate omap tree root node * - * input: Transaction &t, current transaction - * return: return the omap_root_t structure. + * @param Transaction &t, current transaction + * @retval return the omap_root_t structure. */ using initialize_omap_ertr = TransactionManager::alloc_extent_ertr; using initialize_omap_ret = initialize_omap_ertr::future; virtual initialize_omap_ret initialize_omap(Transaction &t) = 0; - /*get value(string) by key(string) + /** + * get value(string) by key(string) * - * input: omap_root_t omap_root, omap btree root information - * input: Transaction &t, current transaction - * input: string &key, omap string key - * return: string key->string value mapping pair. + * @param omap_root_t &omap_root, omap btree root information + * @param Transaction &t, current transaction + * @param string &key, omap string key + * @retval return string key->string value mapping pair. */ using omap_get_value_ertr = TransactionManager::read_extent_ertr; using omap_get_value_ret = omap_get_value_ertr::future>; - virtual omap_get_value_ret omap_get_value(omap_root_t &omap_root, Transaction &t, + virtual omap_get_value_ret omap_get_value(const omap_root_t &omap_root, Transaction &t, const std::string &key) = 0; - /* set key value mapping in omap + /** + * set key value mapping in omap * - * input: omap_root_t &omap_root, omap btree root information - * input: Transaction &t, current transaction - * input: string &key, omap string key - * input: string &value, mapped value corresponding key - * return: mutation_result_t, status should be success. + * @param omap_root_t &omap_root, omap btree root information + * @param Transaction &t, current transaction + * @param string &key, omap string key + * @param string &value, mapped value corresponding key + * @retval mutation_result_t, status should be success. */ using omap_set_key_ertr = TransactionManager::read_extent_ertr; using omap_set_key_ret = omap_set_key_ertr::future; virtual omap_set_key_ret omap_set_key(omap_root_t &omap_root, Transaction &t, const std::string &key, const std::string &value) = 0; - /* remove key value mapping in omap tree + /** + * remove key value mapping in omap tree * - * input: omap_root_t &omap_root, omap btree root information - * input: Transaction &t, current transaction - * input: string &key, omap string key - * return: remove success return true, else return false. + * @param omap_root_t &omap_root, omap btree root information + * @param Transaction &t, current transaction + * @param string &key, omap string key + * @retval remove success return true, else return false. */ using omap_rm_key_ertr = TransactionManager::read_extent_ertr; using omap_rm_key_ret = omap_rm_key_ertr::future; virtual omap_rm_key_ret omap_rm_key(omap_root_t &omap_root, Transaction &t, const std::string &key) = 0; - /* get all keys or partial keys in omap tree + /** + * get all keys or partial keys in omap tree * - * input: omap_root_t &omap_root, omap btree root information - * input: Transaction &t, current transaction - * input: string &start, the list keys range begin from start, + * @param omap_root_t &omap_root, omap btree root information + * @param Transaction &t, current transaction + * @param string &start, the list keys range begin from start, * if start is "", list from the first omap key - * input: max_result_size, the number of list keys, + * @param max_result_size, the number of list keys, * it it is not set, list all keys after string start - * return: list_keys_result_t, listed keys and next key + * @retval list_keys_result_t, listed keys and next key */ using omap_list_keys_ertr = TransactionManager::read_extent_ertr; using omap_list_keys_ret = omap_list_keys_ertr::future; - virtual omap_list_keys_ret omap_list_keys(omap_root_t &omap_root, Transaction &t, + virtual omap_list_keys_ret omap_list_keys(const omap_root_t &omap_root, Transaction &t, std::string &start, size_t max_result_size = MAX_SIZE) = 0; - /* Get all or partial key-> value mapping in omap tree + /** + * Get all or partial key-> value mapping in omap tree * - * input: omap_root_t &omap_root, omap btree root information - * input: Transaction &t, current transaction - * input: string &start, the list keys range begin from start, + * @param omap_root_t &omap_root, omap btree root information + * @param Transaction &t, current transaction + * @param string &start, the list keys range begin from start, * if start is "" , list from the first omap key - * input: max_result_size, the number of list keys, + * @param max_result_size, the number of list keys, * it it is not set, list all keys after string start. - * return: list_kvs_result_t, listed key->value mapping and next key. + * @retval list_kvs_result_t, listed key->value mapping and next key. */ using omap_list_ertr = TransactionManager::read_extent_ertr; using omap_list_ret = omap_list_ertr::future; - virtual omap_list_ret omap_list(omap_root_t &omap_root, Transaction &t, - std::string &start, - size_t max_result_size = MAX_SIZE) = 0; + virtual omap_list_ret omap_list(const omap_root_t &omap_root, Transaction &t, + std::string &start, + size_t max_result_size = MAX_SIZE) = 0; - /* clear all omap tree key->value mapping + /** + * clear all omap tree key->value mapping * - * input: omap_root_t &omap_root, omap btree root information - * input: Transaction &t, current transaction + * @param omap_root_t &omap_root, omap btree root information + * @param Transaction &t, current transaction */ using omap_clear_ertr = TransactionManager::read_extent_ertr; using omap_clear_ret = omap_clear_ertr::future<>; diff --git a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc index 877d192089f..bd4faf13823 100644 --- a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc +++ b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc @@ -37,40 +37,42 @@ BtreeOMapManager::initialize_omap(Transaction &t) } BtreeOMapManager::get_root_ret -BtreeOMapManager::get_omap_root(omap_root_t &omap_root, Transaction &t) +BtreeOMapManager::get_omap_root(const omap_root_t &omap_root, Transaction &t) { assert(omap_root.omap_root_laddr != L_ADDR_NULL); laddr_t laddr = omap_root.omap_root_laddr; - return omap_load_extent(get_omap_context(omap_root, t), laddr, omap_root.depth); + return omap_load_extent(get_omap_context(t), laddr, omap_root.depth); } BtreeOMapManager::handle_root_split_ret -BtreeOMapManager::handle_root_split(omap_context_t oc, OMapNode::mutation_result_t mresult) +BtreeOMapManager::handle_root_split(omap_root_t &omap_root, omap_context_t oc, + OMapNode::mutation_result_t mresult) { return oc.tm.alloc_extent(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE) - .safe_then([oc, mresult](auto&& nroot) { + .safe_then([&omap_root, oc, mresult](auto&& nroot) { auto [left, right, pivot] = *(mresult.split_tuple); - omap_node_meta_t meta{oc.omap_root.depth + 1}; + omap_node_meta_t meta{omap_root.depth + 1}; nroot->set_meta(meta); nroot->journal_inner_insert(nroot->iter_begin(), left->get_laddr(), "", nroot->maybe_get_delta_buffer()); nroot->journal_inner_insert(nroot->iter_begin() + 1, right->get_laddr(), pivot, nroot->maybe_get_delta_buffer()); - oc.omap_root.omap_root_laddr = nroot->get_laddr(); - oc.omap_root.depth += 1; - oc.omap_root.state = omap_root_state_t::MUTATED; + omap_root.omap_root_laddr = nroot->get_laddr(); + omap_root.depth += 1; + omap_root.state = omap_root_state_t::MUTATED; return handle_root_split_ertr::make_ready_future(true); }); } BtreeOMapManager::handle_root_merge_ret -BtreeOMapManager::handle_root_merge(omap_context_t oc, OMapNode::mutation_result_t mresult) +BtreeOMapManager::handle_root_merge(omap_root_t &omap_root, omap_context_t oc, + OMapNode::mutation_result_t mresult) { auto root = *(mresult.need_merge); auto iter = root->cast()->iter_begin(); - oc.omap_root.omap_root_laddr = iter->get_node_key().laddr; - oc.omap_root.depth -= 1; - oc.omap_root.state = omap_root_state_t::MUTATED; + omap_root.omap_root_laddr = iter->get_node_key().laddr; + omap_root.depth -= 1; + omap_root.state = omap_root_state_t::MUTATED; return oc.tm.dec_ref(oc.t, root->get_laddr()).safe_then([] (auto &&ret) { return handle_root_merge_ertr::make_ready_future(true); }); @@ -78,19 +80,18 @@ BtreeOMapManager::handle_root_merge(omap_context_t oc, OMapNode::mutation_result BtreeOMapManager::omap_get_value_ret -BtreeOMapManager::omap_get_value(omap_root_t &omap_root, Transaction &t, +BtreeOMapManager::omap_get_value(const omap_root_t &omap_root, Transaction &t, const std::string &key) { logger().debug("{}: {}", __func__, key); return get_omap_root(omap_root, t).safe_then([this, &omap_root, &t, &key](auto&& extent) { - return extent->get_value(get_omap_context(omap_root, t), key); + return extent->get_value(get_omap_context(t), key); }).safe_then([](auto &&e) { logger().debug("{}: {} -> {}", __func__, e.first, e.second); return omap_get_value_ret( omap_get_value_ertr::ready_future_marker{}, std::move(e)); }); - } BtreeOMapManager::omap_set_key_ret @@ -99,17 +100,15 @@ BtreeOMapManager::omap_set_key(omap_root_t &omap_root, Transaction &t, { logger().debug("{}: {} -> {}", __func__, key, value); return get_omap_root(omap_root, t).safe_then([this, &omap_root, &t, &key, &value](auto root) { - return root->insert(get_omap_context(omap_root, t), key, value); + return root->insert(get_omap_context(t), key, value); }).safe_then([this, &omap_root, &t](auto mresult) { if (mresult.status == mutation_status_t::SUCCESS) return omap_set_key_ertr::make_ready_future(true); - else if (mresult.status == mutation_status_t::SPLITTED) - return handle_root_split(get_omap_context(omap_root, t), mresult); + else if (mresult.status == mutation_status_t::WAS_SPLIT) + return handle_root_split(omap_root, get_omap_context(t), mresult); else return omap_set_key_ertr::make_ready_future(false); - }); - } BtreeOMapManager::omap_rm_key_ret @@ -117,16 +116,16 @@ BtreeOMapManager::omap_rm_key(omap_root_t &omap_root, Transaction &t, const std: { logger().debug("{}: {}", __func__, key); return get_omap_root(omap_root, t).safe_then([this, &omap_root, &t, &key](auto root) { - return root->rm_key(get_omap_context(omap_root, t), key); + return root->rm_key(get_omap_context(t), key); }).safe_then([this, &omap_root, &t](auto mresult) { if (mresult.status == mutation_status_t::SUCCESS) return omap_rm_key_ertr::make_ready_future(true); - else if (mresult.status == mutation_status_t::SPLITTED) - return handle_root_split(get_omap_context(omap_root, t), mresult); + else if (mresult.status == mutation_status_t::WAS_SPLIT) + return handle_root_split(omap_root, get_omap_context(t), mresult); else if (mresult.status == mutation_status_t::NEED_MERGE) { auto root = *(mresult.need_merge); if (root->get_node_size() == 1 && omap_root.depth != 1) - return handle_root_merge(get_omap_context(omap_root, t), mresult); + return handle_root_merge(omap_root, get_omap_context(t), mresult); else return omap_rm_key_ertr::make_ready_future(true); } @@ -137,13 +136,13 @@ BtreeOMapManager::omap_rm_key(omap_root_t &omap_root, Transaction &t, const std: } BtreeOMapManager::omap_list_keys_ret -BtreeOMapManager::omap_list_keys(omap_root_t &omap_root, Transaction &t, +BtreeOMapManager::omap_list_keys(const omap_root_t &omap_root, Transaction &t, std::string &start, size_t max_result_size) { logger().debug("{}", __func__); return get_omap_root(omap_root, t).safe_then([this, &omap_root, &t, &start, max_result_size] (auto extent) { - return extent->list_keys(get_omap_context(omap_root, t), start, max_result_size) + return extent->list_keys(get_omap_context(t), start, max_result_size) .safe_then([](auto &&result) { return omap_list_keys_ret( omap_list_keys_ertr::ready_future_marker{}, @@ -154,13 +153,13 @@ BtreeOMapManager::omap_list_keys(omap_root_t &omap_root, Transaction &t, } BtreeOMapManager::omap_list_ret -BtreeOMapManager::omap_list(omap_root_t &omap_root, Transaction &t, +BtreeOMapManager::omap_list(const omap_root_t &omap_root, Transaction &t, std::string &start, size_t max_result_size) { logger().debug("{}", __func__); return get_omap_root(omap_root, t).safe_then([this, &omap_root, &t, &start, max_result_size] (auto extent) { - return extent->list(get_omap_context(omap_root, t), start, max_result_size) + return extent->list(get_omap_context(t), start, max_result_size) .safe_then([](auto &&result) { return omap_list_ret( omap_list_ertr::ready_future_marker{}, @@ -174,7 +173,7 @@ BtreeOMapManager::omap_clear(omap_root_t &omap_root, Transaction &t) { logger().debug("{}", __func__); return get_omap_root(omap_root, t).safe_then([this, &omap_root, &t](auto extent) { - return extent->clear(get_omap_context(omap_root, t)); + return extent->clear(get_omap_context(t)); }).safe_then([this, &omap_root, &t] { return tm.dec_ref(t, omap_root.omap_root_laddr).safe_then([&omap_root] (auto ret) { omap_root.state = omap_root_state_t::MUTATED; diff --git a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h index d1601bad116..65981fdb049 100644 --- a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h +++ b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h @@ -25,8 +25,8 @@ namespace crimson::os::seastore::omap_manager { class BtreeOMapManager : public OMapManager { TransactionManager &tm; - omap_context_t get_omap_context(omap_root_t &omap_root, Transaction &t) { - return omap_context_t{omap_root, tm, t}; + omap_context_t get_omap_context(Transaction &t) { + return omap_context_t{tm, t}; } /* get_omap_root @@ -35,7 +35,7 @@ class BtreeOMapManager : public OMapManager { */ using get_root_ertr = TransactionManager::read_extent_ertr; using get_root_ret = get_root_ertr::future; - get_root_ret get_omap_root(omap_root_t &omap_root, Transaction &t); + get_root_ret get_omap_root(const omap_root_t &omap_root, Transaction &t); /* handle_root_split * @@ -43,7 +43,7 @@ class BtreeOMapManager : public OMapManager { */ using handle_root_split_ertr = TransactionManager::read_extent_ertr; using handle_root_split_ret = handle_root_split_ertr::future; - handle_root_split_ret handle_root_split(omap_context_t oc, + handle_root_split_ret handle_root_split(omap_root_t &omap_root, omap_context_t oc, OMapNode:: mutation_result_t mresult); /* handle_root_merge @@ -52,7 +52,7 @@ class BtreeOMapManager : public OMapManager { */ using handle_root_merge_ertr = TransactionManager::read_extent_ertr; using handle_root_merge_ret = handle_root_merge_ertr::future; - handle_root_merge_ret handle_root_merge(omap_context_t oc, + handle_root_merge_ret handle_root_merge(omap_root_t &omap_root, omap_context_t oc, OMapNode:: mutation_result_t mresult); public: @@ -60,7 +60,7 @@ public: initialize_omap_ret initialize_omap(Transaction &t) final; - omap_get_value_ret omap_get_value(omap_root_t &omap_root, Transaction &t, + omap_get_value_ret omap_get_value(const omap_root_t &omap_root, Transaction &t, const std::string &key) final; omap_set_key_ret omap_set_key(omap_root_t &omap_root, Transaction &t, @@ -69,11 +69,11 @@ public: omap_rm_key_ret omap_rm_key(omap_root_t &omap_root, Transaction &t, const std::string &key) final; - omap_list_keys_ret omap_list_keys(omap_root_t &omap_root, Transaction &t, + omap_list_keys_ret omap_list_keys(const omap_root_t &omap_root, Transaction &t, std::string &start, size_t max_result_size = MAX_SIZE) final; - omap_list_ret omap_list(omap_root_t &omap_root, Transaction &t, + omap_list_ret omap_list(const omap_root_t &omap_root, Transaction &t, std::string &start, size_t max_result_size = MAX_SIZE) final; diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h index 7a447bb058e..678d6342dfa 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h +++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h @@ -16,14 +16,13 @@ namespace crimson::os::seastore::omap_manager{ struct omap_context_t { - omap_root_t &omap_root; TransactionManager &tm; Transaction &t; }; enum class mutation_status_t : uint8_t { SUCCESS = 0, - SPLITTED = 1, + WAS_SPLIT = 1, NEED_MERGE = 2, FAIL = 3 }; @@ -33,7 +32,7 @@ struct OMapNode : LogicalCachedExtent { struct mutation_result_t { mutation_status_t status; - /// Only populated if SPLITTED, indicates the newly created left and right nodes + /// Only populated if WAS_SPLIT, indicates the newly created left and right nodes /// from splitting the target entry during insertion. std::optional> split_tuple; /// only sopulated if need merged, indicate which entry need be doing merge in upper layer. diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc index b57f66a4e3f..9be588f68ac 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc +++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc @@ -1,8 +1,8 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab +#include #include - #include "include/buffer.h" #include "include/byteorder.h" #include "crimson/os/seastore/transaction_manager.h" @@ -59,7 +59,7 @@ OMapInnerNode::make_split_insert(omap_context_t oc, internal_iterator_t iter, } return make_split_insert_ret( make_split_insert_ertr::ready_future_marker{}, - mutation_result_t(mutation_status_t::SPLITTED, tuple, std::nullopt)); + mutation_result_t(mutation_status_t::WAS_SPLIT, tuple, std::nullopt)); }); } @@ -69,16 +69,17 @@ OMapInnerNode::handle_split_ret OMapInnerNode::handle_split(omap_context_t oc, internal_iterator_t iter, mutation_result_t mresult) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); if (!is_pending()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); auto mut_iter = mut->iter_idx(iter.get_index()); return mut->handle_split(oc, mut_iter, mresult); } auto [left, right, pivot] = *(mresult.split_tuple); - //update will not cause overflow do it first. + //update operation will not cause node overflow, so we can do it first. journal_inner_update(iter, left->get_laddr(), maybe_get_delta_buffer()); - if (!extent_will_overflow(pivot.size() + 1, std::nullopt)) { + bool overflow = extent_will_overflow(pivot.size() + 1, std::nullopt); + if (!overflow) { journal_inner_insert(iter + 1, right->get_laddr(), pivot, maybe_get_delta_buffer()); return insert_ret( @@ -100,7 +101,7 @@ OMapInnerNode::handle_split(omap_context_t oc, internal_iterator_t iter, OMapInnerNode::get_value_ret OMapInnerNode::get_value(omap_context_t oc, const std::string &key) { - logger().debug("{}: {} key = {}", "OMapInnerNode", __func__, key); + logger().debug("OMapInnerNode: {} key = {}", __func__, key); auto child_pt = get_containing_child(key); auto laddr = child_pt->get_node_key().laddr; return omap_load_extent(oc, laddr, get_meta().depth - 1).safe_then( @@ -112,7 +113,7 @@ OMapInnerNode::get_value(omap_context_t oc, const std::string &key) OMapInnerNode::insert_ret OMapInnerNode::insert(omap_context_t oc, const std::string &key, const std::string &value) { - logger().debug("{}: {} {}->{}", "OMapInnerNode", __func__, key, value); + logger().debug("OMapInnerNode: {} {}->{}", __func__, key, value); auto child_pt = get_containing_child(key); assert(child_pt != iter_end()); auto laddr = child_pt->get_node_key().laddr; @@ -122,7 +123,7 @@ OMapInnerNode::insert(omap_context_t oc, const std::string &key, const std::stri }).safe_then([this, oc, child_pt] (auto mresult) { if (mresult.status == mutation_status_t::SUCCESS) { return insert_ertr::make_ready_future(mresult); - } else if (mresult.status == mutation_status_t::SPLITTED) { + } else if (mresult.status == mutation_status_t::WAS_SPLIT) { return handle_split(oc, child_pt, mresult); } else { return insert_ret( @@ -135,28 +136,30 @@ OMapInnerNode::insert(omap_context_t oc, const std::string &key, const std::stri OMapInnerNode::rm_key_ret OMapInnerNode::rm_key(omap_context_t oc, const std::string &key) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); auto child_pt = get_containing_child(key); auto laddr = child_pt->get_node_key().laddr; return omap_load_extent(oc, laddr, get_meta().depth - 1).safe_then( [this, oc, &key, child_pt] (auto extent) { return extent->rm_key(oc, key) .safe_then([this, oc, child_pt, extent = std::move(extent)] (auto mresult) { - if (mresult.status == mutation_status_t::SUCCESS || - mresult.status == mutation_status_t::FAIL) { - return rm_key_ertr::make_ready_future(mresult); - } else if (mresult.status == mutation_status_t::NEED_MERGE) { - if (get_node_size() >1) - return merge_entry(oc, child_pt, *(mresult.need_merge)); - else - return rm_key_ret( - rm_key_ertr::ready_future_marker{}, - mutation_result_t(mutation_status_t::SUCCESS, - std::nullopt, std::nullopt)); - } else if (mresult.status == mutation_status_t::SPLITTED) { - return handle_split(oc, child_pt, mresult); - } else { - return rm_key_ertr::make_ready_future(mresult); + switch (mresult.status) { + case mutation_status_t::SUCCESS: + case mutation_status_t::FAIL: + return rm_key_ertr::make_ready_future(mresult); + case mutation_status_t::NEED_MERGE: { + if (get_node_size() >1) + return merge_entry(oc, child_pt, *(mresult.need_merge)); + else + return rm_key_ret( + rm_key_ertr::ready_future_marker{}, + mutation_result_t(mutation_status_t::SUCCESS, + std::nullopt, std::nullopt)); + } + case mutation_status_t::WAS_SPLIT: + return handle_split(oc, child_pt, mresult); + default: + return rm_key_ertr::make_ready_future(mresult); } }); }); @@ -165,7 +168,7 @@ OMapInnerNode::rm_key(omap_context_t oc, const std::string &key) OMapInnerNode::list_keys_ret OMapInnerNode::list_keys(omap_context_t oc, std::string &start, size_t max_result_size) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); auto child_iter = get_containing_child(start); return seastar::do_with(child_iter, iter_end(), list_keys_result_t(), [=, &start] @@ -179,18 +182,16 @@ OMapInnerNode::list_keys(omap_context_t oc, std::string &start, size_t max_resul auto laddr = biter->get_node_key().laddr; return omap_load_extent(oc, laddr, get_meta().depth - 1).safe_then( [=, &biter, &eiter, &result] (auto &&extent) { - return extent->list_keys(oc, result.next, max_result_size - result.keys.size()) - .safe_then([&biter, &eiter, &result] (auto &&list) mutable { - if (!list.keys.empty()) - result.keys.insert(result.keys.end(), list.keys.begin(),list.keys.end()); - + assert(result.next != std::nullopt); + return extent->list_keys(oc, result.next.value(), max_result_size - result.keys.size()) + .safe_then([&biter, &eiter, &result] (auto &&child_result) mutable { + result.keys.insert(result.keys.end(), child_result.keys.begin(), + child_result.keys.end()); biter++; - if (list.next != "") - result.next = list.next; - else if (biter != eiter) + if (child_result.next == std::nullopt && biter != eiter) result.next = biter->get_node_val(); else - result.next = ""; + result.next = child_result.next; return list_keys_ertr::make_ready_future(false); }); @@ -204,7 +205,7 @@ OMapInnerNode::list_keys(omap_context_t oc, std::string &start, size_t max_resul OMapInnerNode::list_ret OMapInnerNode::list(omap_context_t oc, std::string &start, size_t max_result_size) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); auto child_iter = get_containing_child(start); return seastar::do_with(child_iter, iter_end(), list_kvs_result_t(), [=, &start] @@ -218,18 +219,16 @@ OMapInnerNode::list(omap_context_t oc, std::string &start, size_t max_result_siz auto laddr = biter->get_node_key().laddr; return omap_load_extent(oc, laddr, get_meta().depth - 1).safe_then( [=, &biter, &eiter, &result] (auto &&extent) { - return extent->list(oc, result.next, max_result_size - result.kvs.size()) - .safe_then([&biter, &eiter, &result] (auto &&list) mutable { - if (!list.kvs.empty()) - result.kvs.insert(result.kvs.end(), list.kvs.begin(),list.kvs.end()); - + assert(result.next != std::nullopt); + return extent->list(oc, result.next.value(), max_result_size - result.kvs.size()) + .safe_then([&biter, &eiter, &result] (auto &&child_result) mutable { + result.kvs.insert(result.kvs.end(), child_result.kvs.begin(), + child_result.kvs.end()); biter++; - if (list.next != "") - result.next = list.next; - else if (biter != eiter) + if (child_result.next == std::nullopt && biter != eiter) result.next = biter->get_node_val(); else - result.next = ""; + result.next = child_result.next; return list_ertr::make_ready_future(false); }); @@ -243,7 +242,7 @@ OMapInnerNode::list(omap_context_t oc, std::string &start, size_t max_result_siz OMapInnerNode::clear_ret OMapInnerNode::clear(omap_context_t oc) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); return crimson::do_for_each(iter_begin(), iter_end(), [this, oc] (auto iter) { auto laddr = iter->get_node_key().laddr; return omap_load_extent(oc, laddr, get_meta().depth - 1).safe_then( @@ -260,7 +259,7 @@ OMapInnerNode::clear(omap_context_t oc) OMapInnerNode::split_children_ret OMapInnerNode:: make_split_children(omap_context_t oc) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); return oc.tm.alloc_extents(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE, 2) .safe_then([this] (auto &&ext_pair) { auto left = ext_pair.front(); @@ -274,7 +273,7 @@ OMapInnerNode:: make_split_children(omap_context_t oc) OMapInnerNode::full_merge_ret OMapInnerNode::make_full_merge(omap_context_t oc, OMapNodeRef right) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); return oc.tm.alloc_extent(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE) .safe_then([this, right] (auto &&replacement) { replacement->merge_from(*this, *right->cast()); @@ -287,7 +286,7 @@ OMapInnerNode::make_full_merge(omap_context_t oc, OMapNodeRef right) OMapInnerNode::make_balanced_ret OMapInnerNode::make_balanced(omap_context_t oc, OMapNodeRef _right) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); ceph_assert(_right->get_type() == type); return oc.tm.alloc_extents(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE, 2) .safe_then([this, _right] (auto &&replacement_pair){ @@ -297,15 +296,15 @@ OMapInnerNode::make_balanced(omap_context_t oc, OMapNodeRef _right) return make_balanced_ret( make_balanced_ertr::ready_future_marker{}, std::make_tuple(replacement_left, replacement_right, - balance_into_new_nodes(*this, right, - *replacement_left, *replacement_right))); + balance_into_new_nodes(*this, right, + *replacement_left, *replacement_right))); }); } OMapInnerNode::merge_entry_ret OMapInnerNode::merge_entry(omap_context_t oc, internal_iterator_t iter, OMapNodeRef entry) { - logger().debug("{}: {}","OMapInnerNode", __func__); + logger().debug("OMapInnerNode: {}", __func__); if (!is_pending()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); auto mut_iter = mut->iter_idx(iter->get_index()); @@ -326,13 +325,13 @@ OMapInnerNode::merge_entry(omap_context_t oc, internal_iterator_t iter, OMapNode journal_inner_update(liter, replacement->get_laddr(), maybe_get_delta_buffer()); journal_inner_remove(riter, maybe_get_delta_buffer()); //retire extent - std::list dec_laddrs {l->get_laddr(), r->get_laddr()}; + std::vector dec_laddrs {l->get_laddr(), r->get_laddr()}; return oc.tm.dec_ref(oc.t, dec_laddrs).safe_then([this, oc] (auto &&ret) { if (extent_is_below_min()) { return merge_entry_ret( merge_entry_ertr::ready_future_marker{}, mutation_result_t(mutation_status_t::NEED_MERGE, std::nullopt, - this->cast())); + this)); } else { return merge_entry_ret( merge_entry_ertr::ready_future_marker{}, @@ -344,12 +343,13 @@ OMapInnerNode::merge_entry(omap_context_t oc, internal_iterator_t iter, OMapNode logger().debug("{}::merge_entry balanced l {} r {}", __func__, *l, *r); return l->make_balanced(oc, r).safe_then([=] (auto tuple) { auto [replacement_l, replacement_r, replacement_pivot] = tuple; - //update will not cuase overflow, do it first + //update operation will not cuase node overflow, so we can do it first journal_inner_update(liter, replacement_l->get_laddr(), maybe_get_delta_buffer()); - if (!extent_will_overflow(replacement_pivot.size() + 1, std::nullopt)) { + bool overflow = extent_will_overflow(replacement_pivot.size() + 1, std::nullopt); + if (!overflow) { journal_inner_replace(riter, replacement_r->get_laddr(), replacement_pivot, maybe_get_delta_buffer()); - std::list dec_laddrs{l->get_laddr(), r->get_laddr()}; + std::vector dec_laddrs{l->get_laddr(), r->get_laddr()}; return oc.tm.dec_ref(oc.t, dec_laddrs).safe_then([] (auto &&ret) { return merge_entry_ret( merge_entry_ertr::ready_future_marker{}, @@ -357,11 +357,12 @@ OMapInnerNode::merge_entry(omap_context_t oc, internal_iterator_t iter, OMapNode }); } else { logger().debug("{}::merge_entry balanced and split {} r {}", __func__, *l, *r); - //use remove and insert to instead of replace, remove not cause split do it first + //use remove and insert to instead of replace, + //remove operation will not cause node split, so we can do it first journal_inner_remove(riter, maybe_get_delta_buffer()); return make_split_insert(oc, riter, replacement_pivot, replacement_r->get_laddr()) .safe_then([this, oc, l = l, r = r] (auto mresult) { - std::list dec_laddrs{l->get_laddr(), r->get_laddr(), get_laddr()}; + std::vector dec_laddrs{l->get_laddr(), r->get_laddr(), get_laddr()}; return oc.tm.dec_ref(oc.t, dec_laddrs) .safe_then([mresult = std::move(mresult)] (auto &&ret){ return merge_entry_ret( @@ -379,12 +380,9 @@ OMapInnerNode::merge_entry(omap_context_t oc, internal_iterator_t iter, OMapNode OMapInnerNode::internal_iterator_t OMapInnerNode::get_containing_child(const std::string &key) { - for (auto i = iter_begin(); i != iter_end(); ++i) { - if (i.contains(key)) - return i; - } - ceph_assert( 0 == "invalid"); - return iter_end(); + auto iter = std::find_if(iter_begin(), iter_end(), + [&key](auto it) { return it.contains(key); }); + return iter; } std::ostream &OMapLeafNode::print_detail_l(std::ostream &out) const @@ -396,7 +394,7 @@ std::ostream &OMapLeafNode::print_detail_l(std::ostream &out) const OMapLeafNode::get_value_ret OMapLeafNode::get_value(omap_context_t oc, const std::string &key) { - logger().debug("{}: {} key = {}","OMapLeafNode", __func__, key); + logger().debug("OMapLeafNode: {} key = {}", __func__, key); auto ite = find_string_key(key); if (ite != iter_end()) { auto value = ite->get_string_val(); @@ -413,8 +411,9 @@ OMapLeafNode::get_value(omap_context_t oc, const std::string &key) OMapLeafNode::insert_ret OMapLeafNode::insert(omap_context_t oc, const std::string &key, const std::string &value) { - logger().debug("{}: {}, {} -> {}","OMapLeafNode", __func__, key, value); - if (!extent_will_overflow(key.size() + 1, value.size() + 1)) { + logger().debug("OMapLeafNode: {}, {} -> {}", __func__, key, value); + bool overflow = extent_will_overflow(key.size() + 1, value.size() + 1); + if (!overflow) { if (!is_pending()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); return mut->insert(oc, key, value); @@ -461,7 +460,7 @@ OMapLeafNode::insert(omap_context_t oc, const std::string &key, const std::strin .safe_then([tuple = std::move(tuple)] (auto ret) { return insert_ret( insert_ertr::ready_future_marker{}, - mutation_result_t(mutation_status_t::SPLITTED, tuple, std::nullopt)); + mutation_result_t(mutation_status_t::WAS_SPLIT, tuple, std::nullopt)); }); }); } @@ -470,7 +469,7 @@ OMapLeafNode::insert(omap_context_t oc, const std::string &key, const std::strin OMapLeafNode::rm_key_ret OMapLeafNode::rm_key(omap_context_t oc, const std::string &key) { - logger().debug("{}: {} : {}","OMapLeafNode", __func__, key); + logger().debug("OMapLeafNode: {} : {}", __func__, key); if(!is_pending()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); return mut->rm_key(oc, key); @@ -505,14 +504,15 @@ OMapLeafNode::rm_key(omap_context_t oc, const std::string &key) OMapLeafNode::list_keys_ret OMapLeafNode::list_keys(omap_context_t oc, std::string &start, size_t max_result_size) { - logger().debug("{}: {}","OMapLeafNode", __func__); + logger().debug("OMapLeafNode: {}", __func__); auto result = list_keys_result_t(); iterator iter = start == "" ? iter_begin() : string_lower_bound(start); + // two stop condition, reach the end of leaf or size > required size(max_result_size) for (; iter != iter_end() && result.keys.size() <= max_result_size; iter++) { result.keys.push_back(iter->get_node_val()); } if (iter == iter_end()) - result.next = ""; + result.next = std::nullopt; //have searched all items in the leaf else result.next = iter->get_node_val(); @@ -523,14 +523,14 @@ OMapLeafNode::list_keys(omap_context_t oc, std::string &start, size_t max_result OMapLeafNode::list_ret OMapLeafNode::list(omap_context_t oc, std::string &start, size_t max_result_size) { - logger().debug("{}: {}", "OMapLeafNode", __func__); + logger().debug("OMapLeafNode: {}", __func__); auto result = list_kvs_result_t(); iterator iter = start == "" ? iter_begin() : string_lower_bound(start); for (; iter != iter_end() && result.kvs.size() <= max_result_size; iter++) { result.kvs.push_back({iter->get_node_val(), iter->get_string_val()}); } if (iter == iter_end()) - result.next = ""; + result.next = std::nullopt; //have searched all items in the lead else result.next = iter->get_node_val(); @@ -546,7 +546,7 @@ OMapLeafNode::clear(omap_context_t oc) OMapLeafNode::split_children_ret OMapLeafNode::make_split_children(omap_context_t oc) { - logger().debug("{}: {}","OMapLeafNode", __func__); + logger().debug("OMapLeafNode: {}", __func__); return oc.tm.alloc_extents(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE, 2) .safe_then([this] (auto &&ext_pair) { auto left = ext_pair.front(); @@ -561,7 +561,7 @@ OMapLeafNode::full_merge_ret OMapLeafNode::make_full_merge(omap_context_t oc, OMapNodeRef right) { ceph_assert(right->get_type() == type); - logger().debug("{}: {}","OMapLeafNode", __func__); + logger().debug("OMapLeafNode: {}", __func__); return oc.tm.alloc_extent(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE) .safe_then([this, right] (auto &&replacement) { replacement->merge_from(*this, *right->cast()); @@ -575,7 +575,7 @@ OMapLeafNode::make_balanced_ret OMapLeafNode::make_balanced(omap_context_t oc, OMapNodeRef _right) { ceph_assert(_right->get_type() == type); - logger().debug("{}: {}", "OMapLeafNode", __func__); + logger().debug("OMapLeafNode: {}", __func__); return oc.tm.alloc_extents(oc.t, L_ADDR_MIN, OMAP_BLOCK_SIZE, 2) .safe_then([this, _right] (auto &&replacement_pair) { auto replacement_left = replacement_pair.front(); diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h index 1e0f201f850..126aeaf1512 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h +++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h @@ -100,6 +100,7 @@ struct OMapInnerNode } ceph::bufferlist get_delta() final { + assert(!delta_buffer.empty()); ceph::bufferlist bl; delta_buffer.encode(bl); return bl; @@ -183,6 +184,7 @@ struct OMapLeafNode } ceph::bufferlist get_delta() final { + assert(!delta_buffer.empty()); ceph::bufferlist bl; delta_buffer.encode(bl); return bl; diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_types.h b/src/crimson/os/seastore/omap_manager/btree/omap_types.h index 321d8eb4bcf..c2d52204c44 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_types.h +++ b/src/crimson/os/seastore/omap_manager/btree/omap_types.h @@ -44,12 +44,12 @@ struct omap_node_meta_le_t { }; struct omap_inner_key_t { - uint16_t key_off = 0; - uint16_t key_len = 0; + int16_t key_off = 0; + int16_t key_len = 0; laddr_t laddr = 0; omap_inner_key_t() = default; - omap_inner_key_t(uint16_t off, uint16_t len, laddr_t addr) + omap_inner_key_t(int16_t off, int16_t len, laddr_t addr) : key_off(off), key_len(len), laddr(addr) {} inline bool operator==(const omap_inner_key_t b) const { @@ -65,24 +65,24 @@ struct omap_inner_key_t { }; struct omap_inner_key_le_t { - ceph_le16 key_off = init_le16(0); - ceph_le16 key_len = init_le16(0); + ceph_les16 key_off = init_les16(0); + ceph_les16 key_len = init_les16(0); laddr_le_t laddr = laddr_le_t(0); omap_inner_key_le_t() = default; omap_inner_key_le_t(const omap_inner_key_le_t &) = default; explicit omap_inner_key_le_t(const omap_inner_key_t &key) - : key_off(init_le16(key.key_off)), - key_len(init_le16(key.key_len)), + : key_off(init_les16(key.key_off)), + key_len(init_les16(key.key_len)), laddr(laddr_le_t(key.laddr)) {} operator omap_inner_key_t() const { - return omap_inner_key_t{uint16_t(key_off), uint16_t(key_len), laddr_t(laddr)}; + return omap_inner_key_t{int16_t(key_off), int16_t(key_len), laddr_t(laddr)}; } omap_inner_key_le_t& operator=(omap_inner_key_t key) { - key_off = init_le16(key.key_off); - key_len = init_le16(key.key_len); + key_off = init_les16(key.key_off); + key_len = init_les16(key.key_len); laddr = laddr_le_t(key.laddr); return *this; } @@ -93,13 +93,13 @@ struct omap_inner_key_le_t { }; struct omap_leaf_key_t { - uint16_t key_off = 0; - uint16_t key_len = 0; - uint16_t val_off = 0; - uint16_t val_len = 0; + int16_t key_off = 0; + int16_t key_len = 0; + int16_t val_off = 0; + int16_t val_len = 0; omap_leaf_key_t() = default; - omap_leaf_key_t(uint16_t k_off, uint16_t k_len, uint16_t v_off, uint16_t v_len) + omap_leaf_key_t(int16_t k_off, int16_t k_len, int16_t v_off, int16_t v_len) : key_off(k_off), key_len(k_len), val_off(v_off), val_len(v_len) {} inline bool operator==(const omap_leaf_key_t b) const { @@ -118,29 +118,29 @@ struct omap_leaf_key_t { }; struct omap_leaf_key_le_t { - ceph_le16 key_off = init_le16(0); - ceph_le16 key_len = init_le16(0); - ceph_le16 val_off = init_le16(0); - ceph_le16 val_len = init_le16(0); + ceph_les16 key_off = init_les16(0); + ceph_les16 key_len = init_les16(0); + ceph_les16 val_off = init_les16(0); + ceph_les16 val_len = init_les16(0); omap_leaf_key_le_t() = default; omap_leaf_key_le_t(const omap_leaf_key_le_t &) = default; explicit omap_leaf_key_le_t(const omap_leaf_key_t &key) - : key_off(init_le16(key.key_off)), - key_len(init_le16(key.key_len)), - val_off(init_le16(key.val_off)), - val_len(init_le16(key.val_len)) {} + : key_off(init_les16(key.key_off)), + key_len(init_les16(key.key_len)), + val_off(init_les16(key.val_off)), + val_len(init_les16(key.val_len)) {} operator omap_leaf_key_t() const { - return omap_leaf_key_t{uint16_t(key_off), uint16_t(key_len), - uint16_t(val_off), uint16_t(val_len)}; + return omap_leaf_key_t{int16_t(key_off), int16_t(key_len), + int16_t(val_off), int16_t(val_len)}; } omap_leaf_key_le_t& operator=(omap_leaf_key_t key) { - key_off = init_le16(key.key_off); - key_len = init_le16(key.key_len); - val_off = init_le16(key.val_off); - val_len = init_le16(key.val_len); + key_off = init_les16(key.key_off); + key_len = init_les16(key.key_len); + val_off = init_les16(key.val_off); + val_len = init_les16(key.val_len); return *this; } diff --git a/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h b/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h index 1a278bc7a77..684c4bf3597 100644 --- a/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h +++ b/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h @@ -116,7 +116,7 @@ class StringKVInnerNodeLayout { public: template - struct iter_t { + struct iter_t : public std::iterator { friend class StringKVInnerNodeLayout; template @@ -215,9 +215,9 @@ public: } } - void set_node_val(std::string_view val) { + void set_node_val(const std::string &val) { static_assert(!is_const); - std::strcpy((char*)get_node_val_ptr(), std::string(val).c_str()); //copy char* to char* include "\0" + std::strcpy((char*)get_node_val_ptr(), val.c_str()); //copy char* to char* include "\0" } std::string get_node_val(){ @@ -301,7 +301,7 @@ public: using iterator = iter_t; struct delta_inner_t { - enum class op_t : uint8_t { + enum class op_t : uint_fast8_t { INSERT, UPDATE, REMOVE, @@ -367,7 +367,7 @@ public: }); } void update( - const omap_inner_key_t &key, + const omap_inner_key_t key, std::string_view val) { omap_inner_key_le_t k; k = key; @@ -428,7 +428,7 @@ public: void journal_inner_insert( const_iterator _iter, const laddr_t laddr, - std::string_view key, + const std::string &key, delta_inner_buffer_t *recorder) { auto iter = iterator(this, _iter.index); omap_inner_key_t node_key; @@ -461,7 +461,7 @@ public: void journal_inner_replace( const_iterator _iter, const laddr_t laddr, - std::string_view key, + const std::string &key, delta_inner_buffer_t *recorder) { auto iter = iterator(this, _iter.index); omap_inner_key_t node_key; @@ -803,7 +803,7 @@ private: void inner_insert( iterator iter, const omap_inner_key_t key, - std::string_view val) { + const std::string &val) { if (VALIDATE_INVARIANTS) { if (iter != iter_begin()) { assert((iter - 1)->get_node_val() < val); @@ -830,8 +830,8 @@ private: void inner_replace( iterator iter, - const omap_inner_key_t &key, - std::string_view val) { + const omap_inner_key_t key, + const std::string &val) { assert(iter != iter_end()); if (VALIDATE_INVARIANTS) { if (iter != iter_begin()) { @@ -986,9 +986,9 @@ public: return tail - static_cast(get_node_key().val_off); } - void set_node_val(std::string_view val) const { + void set_node_val(const std::string &val) const { static_assert(!is_const); - std::strcpy((char*)get_node_val_ptr(), std::string(val).c_str()); //copy char* to char* include "\0" + std::strcpy((char*)get_node_val_ptr(), val.c_str()); //copy char* to char* include "\0" } std::string get_node_val() { @@ -1000,9 +1000,9 @@ public: return s; } - void set_string_val(std::string_view val) { + void set_string_val(const std::string &val) { static_assert(!is_const); - std::strcpy((char*)get_string_val_ptr(), std::string(val).c_str()); //copy char* to char* include "\0" + std::strcpy((char*)get_string_val_ptr(), val.c_str()); //copy char* to char* include "\0" } std::string get_string_val() const { @@ -1089,7 +1089,7 @@ public: using iterator = iter_t; struct delta_leaf_t { - enum class op_t : uint8_t { + enum class op_t : uint_fast8_t { INSERT, UPDATE, REMOVE, @@ -1211,8 +1211,8 @@ public: void journal_leaf_insert( const_iterator _iter, - std::string_view key, - std::string_view val, + const std::string &key, + const std::string &val, delta_leaf_buffer_t *recorder) { auto iter = iterator(this, _iter.index); if (recorder) { @@ -1225,8 +1225,8 @@ public: void journal_leaf_update( const_iterator _iter, - std::string_view key, - std::string_view val, + const std::string &key, + const std::string &val, delta_leaf_buffer_t *recorder) { auto iter = iterator(this, _iter.index); if (recorder) { @@ -1562,8 +1562,8 @@ public: private: void leaf_insert( iterator iter, - std::string_view key, - std::string_view val) { + const std::string &key, + const std::string &val) { if (VALIDATE_INVARIANTS) { if (iter != iter_begin()) { assert((iter - 1)->get_node_val() < key); @@ -1596,8 +1596,8 @@ private: void leaf_update( iterator iter, - std::string_view key, - std::string_view val) { + const std::string &key, + const std::string &val) { assert(iter != iter_end()); if (VALIDATE_INVARIANTS) { assert(is_overflow(0, val.size() + 1) == false); diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 63bf46e8e45..bfe5ea23300 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -170,9 +170,9 @@ TransactionManager::ref_ret TransactionManager::dec_ref( TransactionManager::refs_ret TransactionManager::dec_ref( Transaction &t, - std::list offsets) + std::vector offsets) { - return seastar::do_with(std::move(offsets), std::list(), + return seastar::do_with(std::move(offsets), std::vector(), [this, &t] (auto &&offsets, auto &refcnt) { return crimson::do_for_each(offsets.begin(), offsets.end(), [this, &t, &refcnt] (auto &laddr) { @@ -180,7 +180,7 @@ TransactionManager::refs_ret TransactionManager::dec_ref( refcnt.push_back(ref); }); }).safe_then([&refcnt] { - return ref_ertr::make_ready_future>(std::move(refcnt)); + return ref_ertr::make_ready_future>(std::move(refcnt)); }); }); } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 8258a81a9f7..6939335683b 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -178,10 +178,10 @@ public: laddr_t offset); /// remove refcount for list of offset - using refs_ret = ref_ertr::future>; + using refs_ret = ref_ertr::future>; refs_ret dec_ref( Transaction &t, - std::list offsets); + std::vector offsets); /** * alloc_extent diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc index 173d4e6964b..656d9206280 100644 --- a/src/test/crimson/seastore/test_omap_manager.cc +++ b/src/test/crimson/seastore/test_omap_manager.cc @@ -91,6 +91,7 @@ struct omap_manager_test_t : EXPECT_NE(it, test_omap_mappings.end()); EXPECT_EQ(i, it->first); } + EXPECT_EQ(ret.next, std::nullopt); } else { size_t i =0; auto it = test_omap_mappings.find(start); @@ -99,7 +100,7 @@ struct omap_manager_test_t : i++; } if (it == test_omap_mappings.end()) { - EXPECT_EQ(ret.next, ""); + EXPECT_EQ(ret.next, std::nullopt); } else { EXPECT_EQ(ret.keys.size(), max); EXPECT_EQ(ret.next, it->first); @@ -121,6 +122,7 @@ struct omap_manager_test_t : EXPECT_NE(it, test_omap_mappings.end()); EXPECT_EQ(i.second, it->second); } + EXPECT_EQ(ret.next, std::nullopt); } else { size_t i = 0; auto it = test_omap_mappings.find(start); @@ -129,7 +131,7 @@ struct omap_manager_test_t : i++; } if (it == test_omap_mappings.end()) { - EXPECT_EQ(ret.next, ""); + EXPECT_EQ(ret.next, std::nullopt); } else { EXPECT_EQ(ret.kvs.size(), max); EXPECT_EQ(ret.next, it->first); @@ -161,11 +163,7 @@ struct omap_manager_test_t : void replay() { logger().debug("{}: begin", __func__); - tm->close().unsafe_get(); - destroy(); - static_cast(&*segment_manager)->remount(); - init(); - tm->mount().unsafe_get(); + restart(); omap_manager = omap_manager::create_omap_manager(*tm); logger().debug("{}: end", __func__); } -- 2.39.5