From d41a676b494a46ea9bb756f1a9e7eff8a5c36fc6 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Tue, 10 Dec 2024 18:21:53 +0800 Subject: [PATCH] crimson/os/seastore: various cleanups and indention adjustments Signed-off-by: Yingxin Cheng Signed-off-by: Myoungwon Oh --- src/crimson/os/seastore/seastore.cc | 384 ++++++++++++++-------------- src/crimson/os/seastore/seastore.h | 5 +- 2 files changed, 187 insertions(+), 202 deletions(-) diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index 0e9137af1326d..1616e02cdad8f 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -1423,15 +1423,16 @@ SeaStore::Shard::omap_get_values( SeaStore::Shard::_omap_get_value_ret SeaStore::Shard::_omap_get_value( - Transaction &t, - omap_root_t &&root, + Transaction& t, + omap_root_t&& root, std::string_view key) const { return seastar::do_with( BtreeOMapManager(*transaction_manager), std::move(root), std::string(key), - [&t](auto &manager, auto& root, auto& key) -> _omap_get_value_ret { + [&t](auto &manager, auto& root, auto& key) -> _omap_get_value_ret + { LOG_PREFIX(SeaStoreS::_omap_get_value); if (root.is_null()) { DEBUGT("key={} is absent because of null root", t, key); @@ -1451,24 +1452,26 @@ SeaStore::Shard::_omap_get_value( SeaStore::base_iertr::future SeaStore::Shard::_omap_get_values( - Transaction &t, - omap_root_t &&omap_root, - const omap_keys_t &keys) const + Transaction& t, + omap_root_t&& root, + const omap_keys_t& keys) const { LOG_PREFIX(SeaStoreS::_omap_get_values); - if (omap_root.is_null()) { + if (root.is_null()) { DEBUGT("{} keys are absent because of null root", t, keys.size()); return seastar::make_ready_future(); } return seastar::do_with( BtreeOMapManager(*transaction_manager), - std::move(omap_root), + std::move(root), omap_values_t(), - [&t, &keys, FNAME](auto &manager, auto &root, auto &ret) { + [&t, &keys, FNAME](auto &manager, auto &root, auto &ret) + { return trans_intr::do_for_each( keys.begin(), keys.end(), - [&t, &manager, &root, &ret](auto &key) { + [&t, &manager, &root, &ret](auto &key) + { return manager.omap_get_value( root, t, @@ -1493,12 +1496,12 @@ SeaStore::Shard::_omap_get_values( SeaStore::Shard::omap_list_ret SeaStore::Shard::omap_list( Onode &onode, - const omap_root_le_t& omap_root, + const omap_root_le_t& _root, Transaction& t, const std::optional& start, OMapManager::omap_list_config_t config) const { - auto root = omap_root.get( + auto root = _root.get( onode.get_metadata_hint(device->get_block_size())); if (root.is_null()) { return seastar::make_ready_future( @@ -1507,11 +1510,12 @@ SeaStore::Shard::omap_list( } return seastar::do_with( BtreeOMapManager(*transaction_manager), - root, + std::move(root), start, std::optional(std::nullopt), - [&t, config](auto &manager, auto &root, auto &start, auto &end) { - return manager.omap_list(root, t, start, end, config); + [&t, config](auto &manager, auto &root, auto &start, auto &end) + { + return manager.omap_list(root, t, start, end, config); }); } @@ -1520,19 +1524,15 @@ SeaStore::Shard::do_omap_get_values( Transaction& t, Onode& onode, const std::optional& start, - const omap_root_le_t& omap_root) + const omap_root_le_t& root) const { LOG_PREFIX(SeaStoreS::do_omap_get_values); DEBUGT("start={} type={} ...", t, start.has_value() ? *start : "", - omap_root.get_type()); - return omap_list( - onode, - omap_root, - t, - start, - OMapManager::omap_list_config_t() - .with_inclusive(false, false) - .without_max() + root.get_type()); + auto config = OMapManager::omap_list_config_t() + .with_inclusive(false, false) + .without_max(); + return omap_list(onode, root, t, start, config ).si_then([FNAME, &t](omap_values_paged_t ret) { DEBUGT("got {} values, complete={}", t, std::get<1>(ret).size(), std::get<0>(ret)); @@ -2061,26 +2061,21 @@ SeaStore::Shard::_rename( SeaStore::Shard::tm_ret SeaStore::Shard::_remove_omaps( internal_context_t &ctx, - OnodeRef &onode, - omap_root_t &&omap_root) + omap_root_t&& root) { - if (omap_root.get_location() != L_ADDR_NULL) { - return seastar::do_with( - BtreeOMapManager(*transaction_manager), - std::move(omap_root), - [&ctx, onode](auto &omap_manager, auto &omap_root) { - return omap_manager.omap_clear( - omap_root, - *ctx.transaction - ).handle_error_interruptible( - crimson::ct_error::input_output_error::pass_further(), - crimson::ct_error::assert_all{ - "Invalid error in SeaStoreS::_remove_omaps" - } - ); - }); + if (root.is_null()) { + return tm_iertr::now(); } - return tm_iertr::now(); + return seastar::do_with( + BtreeOMapManager(*transaction_manager), + std::move(root), + [&ctx](auto &omap_manager, auto &root) + { + return omap_manager.omap_clear(root, *ctx.transaction + ).si_then([&root] { + assert(root.is_null()); + }); + }); } SeaStore::Shard::tm_ret @@ -2090,33 +2085,31 @@ SeaStore::Shard::_remove( { return _remove_omaps( ctx, - onode, onode->get_layout().omap_root.get( onode->get_metadata_hint(device->get_block_size())) - ).si_then([this, &ctx, onode]() mutable { + ).si_then([this, &ctx, &onode] { return _remove_omaps( ctx, - onode, onode->get_layout().xattr_root.get( onode->get_metadata_hint(device->get_block_size()))); - }).si_then([this, &ctx, onode]() mutable { + }).si_then([this, &ctx, &onode] { return _remove_omaps( ctx, - onode, onode->get_layout().log_root.get( onode->get_metadata_hint(device->get_block_size()))); - }).si_then([this, &ctx, onode] { + }).si_then([this, &ctx, &onode] { return seastar::do_with( ObjectDataHandler(max_object_size), - [=, this, &ctx](auto &objhandler) { - return objhandler.clear( - ObjectDataHandler::context_t{ - *transaction_manager, - *ctx.transaction, - *onode, - }); + [&onode, this, &ctx](auto &objhandler) + { + return objhandler.clear( + ObjectDataHandler::context_t{ + *transaction_manager, + *ctx.transaction, + *onode, + }); }); - }).si_then([this, &ctx, onode]() mutable { + }).si_then([this, &ctx, &onode] { return onode_manager->erase_onode(*ctx.transaction, onode); }).handle_error_interruptible( crimson::ct_error::input_output_error::pass_further(), @@ -2168,20 +2161,22 @@ SeaStore::Shard::_clone_omaps( internal_context_t &ctx, OnodeRef &onode, OnodeRef &d_onode, - const omap_type_t otype) + const omap_type_t type) { - return trans_intr::repeat([&ctx, &onode, &d_onode, this, otype] { + return trans_intr::repeat([&ctx, &onode, &d_onode, this, type] { return seastar::do_with( std::optional(std::nullopt), - [&ctx, &onode, &d_onode, this, otype](auto &start) { - auto& layout = onode->get_layout(); + [&ctx, &onode, &d_onode, this, type](auto &start) + { + auto config = OMapManager::omap_list_config_t() + .with_inclusive(false, false); return omap_list( *onode, - layout.get_root(otype), + onode->get_layout().get_root(type), *ctx.transaction, start, - OMapManager::omap_list_config_t().with_inclusive(false, false) - ).si_then([&ctx, &d_onode, this, otype, &start](auto p) mutable { + config + ).si_then([&ctx, &onode, &d_onode, this, type, &start](auto p) { auto complete = std::get<0>(p); auto &attrs = std::get<1>(p); if (attrs.empty()) { @@ -2195,7 +2190,7 @@ SeaStore::Shard::_clone_omaps( ctx, d_onode, std::map(attrs.begin(), attrs.end()), - d_onode->get_layout().get_root(otype) + d_onode->get_layout().get_root(type) ).si_then([complete, nstart=std::move(nstart), &start]() mutable { if (complete) { @@ -2222,7 +2217,8 @@ SeaStore::Shard::_clone( { return seastar::do_with( ObjectDataHandler(max_object_size), - [this, &ctx, &onode, &d_onode](auto &objHandler) { + [this, &ctx, &onode, &d_onode](auto &objHandler) + { auto &object_size = onode->get_layout().size; d_onode->update_onode_size(*ctx.transaction, object_size); return objHandler.clone( @@ -2232,11 +2228,14 @@ SeaStore::Shard::_clone( *onode, d_onode.get()}); }).si_then([&ctx, &onode, &d_onode, this] { - return _clone_omaps(ctx, onode, d_onode, omap_type_t::XATTR); + return _clone_omaps( + ctx, onode, d_onode, omap_type_t::XATTR); }).si_then([&ctx, &onode, &d_onode, this] { - return _clone_omaps(ctx, onode, d_onode, omap_type_t::OMAP); + return _clone_omaps( + ctx, onode, d_onode, omap_type_t::OMAP); }).si_then([&ctx, &onode, &d_onode, this] { - return _clone_omaps(ctx, onode, d_onode, omap_type_t::LOG); + return _clone_omaps( + ctx, onode, d_onode, omap_type_t::LOG); }); } @@ -2276,12 +2275,12 @@ SeaStore::Shard::_omap_set_values( internal_context_t &ctx, OnodeRef &onode, std::map &&kvs, - const omap_root_le_t &omap_root) + const omap_root_le_t &_root) { Transaction& t = *ctx.transaction; return seastar::do_with( BtreeOMapManager(*transaction_manager), - omap_root.get(onode->get_metadata_hint(device->get_block_size())), + _root.get(onode->get_metadata_hint(device->get_block_size())), [this, &t, &onode, kvs=std::move(kvs)] (auto &omap_manager, auto &root) mutable { @@ -2324,7 +2323,7 @@ SeaStore::Shard::_omap_set_header( { std::map to_set; to_set[OMAP_HEADER_XATTR_KEY] = header; - return _setattrs(ctx, onode,std::move(to_set)); + return _setattrs(ctx, onode, std::move(to_set)); } SeaStore::Shard::tm_ret @@ -2332,30 +2331,28 @@ SeaStore::Shard::_omap_clear( internal_context_t &ctx, OnodeRef &onode) { - return _xattr_rmattr(ctx, onode, std::string(OMAP_HEADER_XATTR_KEY) - ).si_then([this, &ctx, &onode]() -> tm_ret { - if (auto omap_root = onode->get_layout().omap_root.get( + return _xattr_rmattr( + ctx, + onode, + std::string(OMAP_HEADER_XATTR_KEY) + ).si_then([this, &ctx, &onode] { + auto root = onode->get_layout().omap_root.get( onode->get_metadata_hint(device->get_block_size())); - omap_root.is_null()) { - return seastar::now(); - } else { - return seastar::do_with( - BtreeOMapManager(*transaction_manager), - onode->get_layout().omap_root.get( - onode->get_metadata_hint(device->get_block_size())), - [&ctx, &onode]( - auto &omap_manager, - auto &omap_root) { - return omap_manager.omap_clear( - omap_root, - *ctx.transaction - ).si_then([&] { - if (omap_root.must_update()) { - onode->update_omap_root(*ctx.transaction, omap_root); - } - }); - }); + if (root.is_null()) { + return base_iertr::now(); } + return seastar::do_with( + BtreeOMapManager(*transaction_manager), + std::move(root), + [&ctx, &onode](auto &omap_manager, auto &root) + { + return omap_manager.omap_clear(root, *ctx.transaction + ).si_then([&ctx, &onode, &root] { + assert(root.is_null()); + assert(root.must_update()); + onode->update_omap_root(*ctx.transaction, root); + }); + }); }); } @@ -2364,44 +2361,37 @@ SeaStore::Shard::_omap_rmkeys( internal_context_t &ctx, OnodeRef &onode, omap_keys_t &&keys, - const omap_root_le_t &_omap_root) + const omap_root_le_t &_root) { - auto omap_root = _omap_root.get( + auto root = _root.get( onode->get_metadata_hint(device->get_block_size())); - if (omap_root.is_null()) { + if (root.is_null()) { return seastar::now(); - } else { - return seastar::do_with( - BtreeOMapManager(*transaction_manager), - _omap_root.get( - onode->get_metadata_hint(device->get_block_size())), - std::move(keys), - [&ctx, &onode]( - auto &omap_manager, - auto &omap_root, - auto &keys) { - return trans_intr::do_for_each( - keys.begin(), - keys.end(), - [&](auto &p) { - return omap_manager.omap_rm_key( - omap_root, - *ctx.transaction, - p); - } - ).si_then([&] { - if (omap_root.must_update()) { - if (omap_root.get_type() == omap_type_t::OMAP) { - onode->update_omap_root(*ctx.transaction, omap_root); - } else { - ceph_assert(omap_root.get_type() == omap_type_t::LOG); - onode->update_log_root(*ctx.transaction, omap_root); - } - } - }); - } - ); } + return seastar::do_with( + BtreeOMapManager(*transaction_manager), + std::move(root), + std::move(keys), + [&ctx, &onode] + (auto &omap_manager, auto &root, auto &keys) + { + return trans_intr::do_for_each( + keys.begin(), + keys.end(), + [&omap_manager, &ctx, &root](auto &p) + { + return omap_manager.omap_rm_key(root, *ctx.transaction, p); + }).si_then([&ctx, &root, &onode] { + if (root.must_update()) { + if (root.get_type() == omap_type_t::OMAP) { + onode->update_omap_root(*ctx.transaction, root); + } else { + ceph_assert(root.get_type() == omap_type_t::LOG); + onode->update_log_root(*ctx.transaction, root); + } + } + }); + }); } SeaStore::Shard::tm_ret @@ -2410,50 +2400,42 @@ SeaStore::Shard::_omap_rmkeyrange( OnodeRef &onode, std::string first, std::string last, - const omap_root_le_t &_omap_root) + const omap_root_le_t &_root) { if (first > last) { LOG_PREFIX(SeaStoreS::_omap_rmkeyrange); ERRORT("range error, first:{} > last:{}", *ctx.transaction, first, last); ceph_abort(); } - auto omap_root = _omap_root.get( + auto root = _root.get( onode->get_metadata_hint(device->get_block_size())); - if (omap_root.is_null()) { + if (root.is_null()) { return seastar::now(); - } else { - return seastar::do_with( - BtreeOMapManager(*transaction_manager), - _omap_root.get( - onode->get_metadata_hint(device->get_block_size())), - std::move(first), - std::move(last), - [&ctx, &onode]( - auto &omap_manager, - auto &omap_root, - auto &first, - auto &last) { - auto config = OMapManager::omap_list_config_t() - .with_inclusive(true, false) - .without_max(); - return omap_manager.omap_rm_key_range( - omap_root, - *ctx.transaction, - first, - last, - config - ).si_then([&] { - if (omap_root.must_update()) { - if (omap_root.get_type() == omap_type_t::OMAP) { - onode->update_omap_root(*ctx.transaction, omap_root); - } else { - ceph_assert(omap_root.get_type() == omap_type_t::LOG); - onode->update_log_root(*ctx.transaction, omap_root); - } + } + return seastar::do_with( + BtreeOMapManager(*transaction_manager), + std::move(root), + std::move(first), + std::move(last), + [&ctx, &onode] + (auto &omap_manager, auto &root, auto &first, auto &last) + { + auto config = OMapManager::omap_list_config_t() + .with_inclusive(true, false) + .without_max(); + return omap_manager.omap_rm_key_range( + root, *ctx.transaction, first, last, config + ).si_then([&ctx, &root, &onode] { + if (root.must_update()) { + if (root.get_type() == omap_type_t::OMAP) { + onode->update_omap_root(*ctx.transaction, root); + } else { + ceph_assert(root.get_type() == omap_type_t::LOG); + onode->update_log_root(*ctx.transaction, root); } - }); + } }); - } + }); } SeaStore::Shard::tm_ret @@ -2492,7 +2474,10 @@ SeaStore::Shard::_setattrs( if (!layout.oi_size) { // if oi was not in the layout, it probably exists in the omap, // need to remove it first - fut = _xattr_rmattr(ctx, onode, OI_ATTR); + fut = _xattr_rmattr( + ctx, + onode, + OI_ATTR); } onode->update_object_info(*ctx.transaction, val); aset.erase(it); @@ -2507,7 +2492,10 @@ SeaStore::Shard::_setattrs( if (likely(val.length() <= onode_layout_t::MAX_SS_LENGTH)) { if (!layout.ss_size) { - fut = _xattr_rmattr(ctx, onode, SS_ATTR); + fut = _xattr_rmattr( + ctx, + onode, + SS_ATTR); } onode->update_snapset(*ctx.transaction, val); aset.erase(it); @@ -2523,8 +2511,8 @@ SeaStore::Shard::_setattrs( } DEBUGT("set attrs in omap", *ctx.transaction); - return fut.si_then( - [this, onode, &ctx, aset=std::move(aset)]() mutable { + return std::move(fut + ).si_then([this, onode, &ctx, aset=std::move(aset)]() mutable { return _omap_set_values( ctx, onode, @@ -2562,25 +2550,24 @@ SeaStore::Shard::_xattr_rmattr( { LOG_PREFIX(SeaStoreS::_xattr_rmattr); DEBUGT("onode={}", *ctx.transaction, *onode); - auto xattr_root = onode->get_layout().xattr_root.get( + auto root = onode->get_layout().xattr_root.get( onode->get_metadata_hint(device->get_block_size())); - if (xattr_root.is_null()) { - return seastar::now(); - } else { - return seastar::do_with( - BtreeOMapManager(*transaction_manager), - onode->get_layout().xattr_root.get( - onode->get_metadata_hint(device->get_block_size())), - std::move(name), - [&ctx, &onode](auto &omap_manager, auto &xattr_root, auto &name) { - return omap_manager.omap_rm_key(xattr_root, *ctx.transaction, name) - .si_then([&] { - if (xattr_root.must_update()) { - onode->update_xattr_root(*ctx.transaction, xattr_root); - } - }); - }); + if (root.is_null()) { + return base_iertr::now(); } + return seastar::do_with( + BtreeOMapManager(*transaction_manager), + std::move(root), + std::move(name), + [&ctx, &onode](auto &omap_manager, auto &root, auto &name) + { + return omap_manager.omap_rm_key(root, *ctx.transaction, name + ).si_then([&ctx, &root, &onode] { + if (root.must_update()) { + onode->update_xattr_root(*ctx.transaction, root); + } + }); + }); } SeaStore::Shard::tm_ret @@ -2600,24 +2587,23 @@ SeaStore::Shard::_xattr_clear( { LOG_PREFIX(SeaStoreS::_xattr_clear); DEBUGT("onode={}", *ctx.transaction, *onode); - auto xattr_root = onode->get_layout().xattr_root.get( + auto root = onode->get_layout().xattr_root.get( onode->get_metadata_hint(device->get_block_size())); - if (xattr_root.is_null()) { - return seastar::now(); - } else { - return seastar::do_with( - BtreeOMapManager(*transaction_manager), - onode->get_layout().xattr_root.get( - onode->get_metadata_hint(device->get_block_size())), - [&ctx, &onode](auto &omap_manager, auto &xattr_root) { - return omap_manager.omap_clear(xattr_root, *ctx.transaction) - .si_then([&] { - if (xattr_root.must_update()) { - onode->update_xattr_root(*ctx.transaction, xattr_root); - } - }); - }); + if (root.is_null()) { + return base_iertr::now(); } + return seastar::do_with( + BtreeOMapManager(*transaction_manager), + std::move(root), + [&ctx, &onode](auto &omap_manager, auto &root) + { + return omap_manager.omap_clear(root, *ctx.transaction + ).si_then([&ctx, &onode, &root] { + assert(root.is_null()); + assert(root.must_update()); + onode->update_xattr_root(*ctx.transaction, root); + }); + }); } SeaStore::Shard::tm_ret diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h index dbefc1f53f68d..f5e010de8dec9 100644 --- a/src/crimson/os/seastore/seastore.h +++ b/src/crimson/os/seastore/seastore.h @@ -398,7 +398,7 @@ public: Transaction& t, Onode& onode, const std::optional& start, - const omap_root_le_t& omap_root); + const omap_root_le_t& omap_root) const; base_iertr::future _fiemap( Transaction &t, @@ -416,7 +416,6 @@ public: tm_ret _remove_omaps( internal_context_t &ctx, - OnodeRef &onode, omap_root_t &&omap_root); tm_ret _remove( internal_context_t &ctx, @@ -517,7 +516,7 @@ public: seastar::metrics::histogram& lat = get_latency(op_type); lat.sample_count++; lat.sample_sum += std::chrono::duration_cast(dur).count(); - } + } private: std::string root; -- 2.39.5