From 0690cc08d4097866e0061d95480ab33f3b07327a Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 19 Aug 2020 00:57:58 +0800 Subject: [PATCH] crimson/osd: errorator omap_get_values() ops * os/alienstore: do not panic if retval is less than 0. instead, translate ENOENT to enoent and other erros to input_output_error * os/FuturizedStore: errorator omap_get_values() variants, so they return errorator::future<> instead of plain seastar::future<> * os/seastore: update accordingly. * osd/pg_backend: use enodata for OI without omap in it. better performance, and better readability this way. Signed-off-by: Kefu Chai --- src/crimson/os/alienstore/alien_store.cc | 44 +++++++++----- src/crimson/os/alienstore/alien_store.h | 16 +++--- src/crimson/os/cyanstore/cyan_store.cc | 31 +++++----- src/crimson/os/cyanstore/cyan_store.h | 16 +++--- src/crimson/os/futurized_store.h | 10 ++-- src/crimson/os/seastore/seastore.cc | 36 ++++++------ src/crimson/os/seastore/seastore.h | 16 +++--- src/crimson/osd/pg_backend.cc | 73 +++++++++++++++++------- src/crimson/osd/pg_backend.h | 6 +- src/crimson/osd/pg_meta.cc | 11 +++- 10 files changed, 155 insertions(+), 104 deletions(-) diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc index 7ce7af21f979..f89acf613fb6 100644 --- a/src/crimson/os/alienstore/alien_store.cc +++ b/src/crimson/os/alienstore/alien_store.cc @@ -297,7 +297,7 @@ AlienStore::get_attrs(CollectionRef ch, reinterpret_cast&>(aset)); }).then([&aset] (int r) -> get_attrs_ertr::future { if (r == -ENOENT) { - return crimson::ct_error::enoent::make();; + return crimson::ct_error::enoent::make(); } else { return get_attrs_ertr::make_ready_future(std::move(aset)); } @@ -305,10 +305,10 @@ AlienStore::get_attrs(CollectionRef ch, }); } -seastar::future -AlienStore::omap_get_values(CollectionRef ch, - const ghobject_t& oid, - const set& keys) +auto AlienStore::omap_get_values(CollectionRef ch, + const ghobject_t& oid, + const set& keys) + -> read_errorator::future { logger().debug("{}", __func__); return seastar::do_with(omap_values_t{}, [=] (auto &values) { @@ -316,17 +316,23 @@ AlienStore::omap_get_values(CollectionRef ch, auto c = static_cast(ch.get()); return store->omap_get_values(c->collection, oid, keys, reinterpret_cast*>(&values)); - }).then([&values] (int r) { - assert(r == 0); - return seastar::make_ready_future(std::move(values)); + }).then([&values] (int r) -> read_errorator::future { + if (r == -ENOENT) { + return crimson::ct_error::enoent::make(); + } else if (r < 0){ + logger().error("omap_get_values: {}", r); + return crimson::ct_error::input_output_error::make(); + } else { + return read_errorator::make_ready_future(std::move(values)); + } }); }); } -seastar::future> -AlienStore::omap_get_values(CollectionRef ch, - const ghobject_t &oid, - const std::optional &start) +auto AlienStore::omap_get_values(CollectionRef ch, + const ghobject_t &oid, + const std::optional &start) + -> read_errorator::future> { logger().debug("{} with_start", __func__); return seastar::do_with(omap_values_t{}, [=] (auto &values) { @@ -334,9 +340,17 @@ AlienStore::omap_get_values(CollectionRef ch, auto c = static_cast(ch.get()); return store->omap_get_values(c->collection, oid, start, reinterpret_cast*>(&values)); - }).then([&values] (int r) { - return seastar::make_ready_future>( - std::make_tuple(true, std::move(values))); + }).then([&values] (int r) + -> read_errorator::future> { + if (r == -ENOENT) { + return crimson::ct_error::enoent::make(); + } else if (r < 0){ + logger().error("omap_get_values(start): {}", r); + return crimson::ct_error::input_output_error::make(); + } else { + return read_errorator::make_ready_future>( + std::make_tuple(true, std::move(values))); + } }); }); } diff --git a/src/crimson/os/alienstore/alien_store.h b/src/crimson/os/alienstore/alien_store.h index c9ecfdf4e316..55d0cda55834 100644 --- a/src/crimson/os/alienstore/alien_store.h +++ b/src/crimson/os/alienstore/alien_store.h @@ -64,24 +64,24 @@ public: get_attrs_ertr::future get_attrs(CollectionRef c, const ghobject_t& oid) final; - seastar::future omap_get_values( + read_errorator::future omap_get_values( CollectionRef c, const ghobject_t& oid, const omap_keys_t& keys) final; - seastar::future, ghobject_t>> list_objects( - CollectionRef c, - const ghobject_t& start, - const ghobject_t& end, - uint64_t limit) const final; - /// Retrieves paged set of values > start (if present) - seastar::future> omap_get_values( + read_errorator::future> omap_get_values( CollectionRef c, ///< [in] collection const ghobject_t &oid, ///< [in] oid const std::optional &start ///< [in] start, empty for begin ) final; ///< @return values.empty() iff done + seastar::future, ghobject_t>> list_objects( + CollectionRef c, + const ghobject_t& start, + const ghobject_t& end, + uint64_t limit) const final; + seastar::future create_new_collection(const coll_t& cid) final; seastar::future open_collection(const coll_t& cid) final; seastar::future> list_collections() final; diff --git a/src/crimson/os/cyanstore/cyan_store.cc b/src/crimson/os/cyanstore/cyan_store.cc index 4ba19f67effb..57363b1d81ab 100644 --- a/src/crimson/os/cyanstore/cyan_store.cc +++ b/src/crimson/os/cyanstore/cyan_store.cc @@ -253,17 +253,16 @@ CyanStore::get_attrs_ertr::future CyanStore::get_attrs( return get_attrs_ertr::make_ready_future(o->xattr); } -seastar::future -CyanStore::omap_get_values(CollectionRef ch, - const ghobject_t& oid, - const omap_keys_t& keys) +auto CyanStore::omap_get_values(CollectionRef ch, + const ghobject_t& oid, + const omap_keys_t& keys) + -> read_errorator::future { auto c = static_cast(ch.get()); - logger().debug("{} {} {}", - __func__, c->get_cid(), oid); + logger().debug("{} {} {}", __func__, c->get_cid(), oid); auto o = c->get_object(oid); if (!o) { - throw std::runtime_error(fmt::format("object does not exist: {}", oid)); + return crimson::ct_error::enoent::make(); } omap_values_t values; for (auto& key : keys) { @@ -274,19 +273,17 @@ CyanStore::omap_get_values(CollectionRef ch, return seastar::make_ready_future(std::move(values)); } -seastar::future> -CyanStore::omap_get_values( - CollectionRef ch, - const ghobject_t &oid, - const std::optional &start - ) { +auto +CyanStore::omap_get_values(CollectionRef ch, + const ghobject_t &oid, + const std::optional &start) + -> read_errorator::future> +{ auto c = static_cast(ch.get()); - logger().debug( - "{} {} {}", - __func__, c->get_cid(), oid); + logger().debug("{} {} {}", __func__, c->get_cid(), oid); auto o = c->get_object(oid); if (!o) { - throw std::runtime_error(fmt::format("object does not exist: {}", oid)); + return crimson::ct_error::enoent::make(); } omap_values_t values; for (auto i = start ? o->omap.upper_bound(*start) : o->omap.begin(); diff --git a/src/crimson/os/cyanstore/cyan_store.h b/src/crimson/os/cyanstore/cyan_store.h index a53cfea93157..877400272b56 100644 --- a/src/crimson/os/cyanstore/cyan_store.h +++ b/src/crimson/os/cyanstore/cyan_store.h @@ -100,24 +100,24 @@ public: CollectionRef c, const ghobject_t& oid); - seastar::future omap_get_values( + read_errorator::future omap_get_values( CollectionRef c, const ghobject_t& oid, const omap_keys_t& keys) final; - seastar::future, ghobject_t>> list_objects( - CollectionRef c, - const ghobject_t& start, - const ghobject_t& end, - uint64_t limit) const final; - /// Retrieves paged set of values > start (if present) - seastar::future> omap_get_values( + read_errorator::future> omap_get_values( CollectionRef c, ///< [in] collection const ghobject_t &oid, ///< [in] oid const std::optional &start ///< [in] start, empty for begin ) final; ///< @return values.empty() iff done + seastar::future, ghobject_t>> list_objects( + CollectionRef c, + const ghobject_t& start, + const ghobject_t& end, + uint64_t limit) const final; + seastar::future omap_get_header( CollectionRef c, const ghobject_t& oid) final; diff --git a/src/crimson/os/futurized_store.h b/src/crimson/os/futurized_store.h index 445f34ef4dcb..dd41f9c04934 100644 --- a/src/crimson/os/futurized_store.h +++ b/src/crimson/os/futurized_store.h @@ -118,16 +118,16 @@ public: using omap_values_t = std::map>; using omap_keys_t = std::set; - virtual seastar::future omap_get_values( - CollectionRef c, - const ghobject_t& oid, - const omap_keys_t& keys) = 0; + virtual read_errorator::future omap_get_values( + CollectionRef c, + const ghobject_t& oid, + const omap_keys_t& keys) = 0; virtual seastar::future, ghobject_t>> list_objects( CollectionRef c, const ghobject_t& start, const ghobject_t& end, uint64_t limit) const = 0; - virtual seastar::future> omap_get_values( + virtual read_errorator::future> omap_get_values( CollectionRef c, ///< [in] collection const ghobject_t &oid, ///< [in] oid const std::optional &start ///< [in] start, empty for begin diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index bae0bd857ddc..e9d77f7228bc 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -163,18 +163,6 @@ seastar::future SeaStore::stat( return seastar::make_ready_future(st); } -seastar::future -SeaStore::omap_get_values(CollectionRef ch, - const ghobject_t& oid, - const omap_keys_t& keys) -{ - auto c = static_cast(ch.get()); - logger().debug("{} {} {}", - __func__, c->get_cid(), oid); - return seastar::make_ready_future(); -} - - seastar::future omap_get_header( CollectionRef c, const ghobject_t& oid) @@ -182,12 +170,26 @@ seastar::future omap_get_header( return seastar::make_ready_future(); } -seastar::future> +auto +SeaStore::omap_get_values( + CollectionRef ch, + const ghobject_t& oid, + const omap_keys_t& keys) + -> read_errorator::future +{ + auto c = static_cast(ch.get()); + logger().debug("{} {} {}", + __func__, c->get_cid(), oid); + return seastar::make_ready_future(); +} + +auto SeaStore::omap_get_values( - CollectionRef ch, - const ghobject_t &oid, - const std::optional &start - ) { + CollectionRef ch, + const ghobject_t &oid, + const std::optional &start) + -> read_errorator::future> +{ auto c = static_cast(ch.get()); logger().debug( "{} {} {}", diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h index 38afac2e6476..5ca383c81027 100644 --- a/src/crimson/os/seastore/seastore.h +++ b/src/crimson/os/seastore/seastore.h @@ -69,11 +69,18 @@ public: CollectionRef c, const ghobject_t& oid) final; - seastar::future omap_get_values( + read_errorator::future omap_get_values( CollectionRef c, const ghobject_t& oid, const omap_keys_t& keys) final; + /// Retrieves paged set of values > start (if present) + read_errorator::future> omap_get_values( + CollectionRef c, ///< [in] collection + const ghobject_t &oid, ///< [in] oid + const std::optional &start ///< [in] start, empty for begin + ) final; ///< @return values.empty() iff done + seastar::future omap_get_header( CollectionRef c, const ghobject_t& oid) final; @@ -84,13 +91,6 @@ public: const ghobject_t& end, uint64_t limit) const final; - /// Retrieves paged set of values > start (if present) - seastar::future> omap_get_values( - CollectionRef c, ///< [in] collection - const ghobject_t &oid, ///< [in] oid - const std::optional &start ///< [in] start, empty for begin - ) final; ///< @return values.empty() iff done - seastar::future create_new_collection(const coll_t& cid) final; seastar::future open_collection(const coll_t& cid) final; seastar::future> list_collections() final; diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc index 3a35ffc2f2cd..eb3defaacec2 100644 --- a/src/crimson/osd/pg_backend.cc +++ b/src/crimson/osd/pg_backend.cc @@ -723,7 +723,12 @@ PGBackend::get_attr_errorator::future<> PGBackend::get_xattrs( }); } -static seastar::future +using get_omap_ertr = + crimson::os::FuturizedStore::read_errorator::extend< + crimson::ct_error::enodata>; +static +get_omap_ertr::future< + crimson::os::FuturizedStore::omap_values_t> maybe_get_omap_vals_by_keys( crimson::os::FuturizedStore* store, const crimson::os::CollectionRef& coll, @@ -733,12 +738,13 @@ maybe_get_omap_vals_by_keys( if (oi.is_omap()) { return store->omap_get_values(coll, ghobject_t{oi.soid}, keys_to_get); } else { - return seastar::make_ready_future( - crimson::os::FuturizedStore::omap_values_t{}); + return crimson::ct_error::enodata::make(); } } -static seastar::future> +static +get_omap_ertr::future< + std::tuple> maybe_get_omap_vals( crimson::os::FuturizedStore* store, const crimson::os::CollectionRef& coll, @@ -748,8 +754,7 @@ maybe_get_omap_vals( if (oi.is_omap()) { return store->omap_get_values(coll, ghobject_t{oi.soid}, start_after); } else { - return seastar::make_ready_future>( - std::make_tuple(true, crimson::os::FuturizedStore::omap_values_t{})); + return crimson::ct_error::enodata::make(); } } @@ -771,7 +776,8 @@ seastar::future<> PGBackend::omap_get_header( }); } -seastar::future<> PGBackend::omap_get_keys( +PGBackend::ll_read_errorator::future<> +PGBackend::omap_get_keys( const ObjectState& os, OSDOp& osd_op) const { @@ -793,12 +799,12 @@ seastar::future<> PGBackend::omap_get_keys( std::min(max_return, local_conf()->osd_max_omap_entries_per_request); // TODO: truly chunk the reading - return maybe_get_omap_vals(store, coll, os.oi, start_after).then( - [=, &osd_op] (auto ret) { + return maybe_get_omap_vals(store, coll, os.oi, start_after).safe_then( + [=, &osd_op](auto ret) { ceph::bufferlist result; bool truncated = false; uint32_t num = 0; - for (auto& [key, val] : std::get<1>(ret)) { + for (auto &[key, val] : std::get<1>(ret)) { if (num >= max_return || result.length() >= local_conf()->osd_max_omap_bytes_per_request) { truncated = true; @@ -811,14 +817,23 @@ seastar::future<> PGBackend::omap_get_keys( osd_op.outdata.claim_append(result); encode(truncated, osd_op.outdata); return seastar::now(); - }); - + }).handle_error( + crimson::ct_error::enodata::handle([&osd_op] { + uint32_t num = 0; + bool truncated = false; + encode(num, osd_op.outdata); + encode(truncated, osd_op.outdata); + return seastar::now(); + }), + ll_read_errorator::pass_further{} + ); // TODO: //ctx->delta_stats.num_rd_kb += shift_round_up(osd_op.outdata.length(), 10); //ctx->delta_stats.num_rd++; } -seastar::future<> PGBackend::omap_get_vals( +PGBackend::ll_read_errorator::future<> +PGBackend::omap_get_vals( const ObjectState& os, OSDOp& osd_op) const { @@ -843,7 +858,7 @@ seastar::future<> PGBackend::omap_get_vals( std::min(max_return, local_conf()->osd_max_omap_entries_per_request); // TODO: truly chunk the reading - return maybe_get_omap_vals(store, coll, os.oi, start_after).then( + return maybe_get_omap_vals(store, coll, os.oi, start_after).safe_then( [=, &osd_op] (auto&& ret) { auto [done, vals] = std::move(ret); assert(done); @@ -868,14 +883,23 @@ seastar::future<> PGBackend::omap_get_vals( encode(num, osd_op.outdata); osd_op.outdata.claim_append(result); encode(truncated, osd_op.outdata); - return seastar::now(); - }); + return ll_read_errorator::now(); + }).handle_error( + crimson::ct_error::enodata::handle([&osd_op] { + encode(uint32_t{0} /* num */, osd_op.outdata); + encode(bool{false} /* truncated */, osd_op.outdata); + return ll_read_errorator::now(); + }), + ll_read_errorator::pass_further{} + ); // TODO: //ctx->delta_stats.num_rd_kb += shift_round_up(osd_op.outdata.length(), 10); //ctx->delta_stats.num_rd++; } -seastar::future<> PGBackend::omap_get_vals_by_keys( + +PGBackend::ll_read_errorator::future<> +PGBackend::omap_get_vals_by_keys( const ObjectState& os, OSDOp& osd_op) const { @@ -892,11 +916,18 @@ seastar::future<> PGBackend::omap_get_vals_by_keys( throw crimson::osd::invalid_argument(); } - return maybe_get_omap_vals_by_keys(store, coll, os.oi, keys_to_get).then( - [&osd_op] (crimson::os::FuturizedStore::omap_values_t vals) { + return maybe_get_omap_vals_by_keys(store, coll, os.oi, keys_to_get).safe_then( + [&osd_op] (crimson::os::FuturizedStore::omap_values_t&& vals) { encode(vals, osd_op.outdata); - return seastar::now(); - }); + return ll_read_errorator::now(); + }).handle_error( + crimson::ct_error::enodata::handle([&osd_op] { + uint32_t num = 0; + encode(num, osd_op.outdata); + return ll_read_errorator::now(); + }), + ll_read_errorator::pass_further{} + ); // TODO: //ctx->delta_stats.num_rd_kb += shift_round_up(osd_op.outdata.length(), 10); diff --git a/src/crimson/osd/pg_backend.h b/src/crimson/osd/pg_backend.h index dab750dde17f..39516a7f04ac 100644 --- a/src/crimson/osd/pg_backend.h +++ b/src/crimson/osd/pg_backend.h @@ -143,13 +143,13 @@ public: uint64_t len); // OMAP - seastar::future<> omap_get_keys( + ll_read_errorator::future<> omap_get_keys( const ObjectState& os, OSDOp& osd_op) const; - seastar::future<> omap_get_vals( + ll_read_errorator::future<> omap_get_vals( const ObjectState& os, OSDOp& osd_op) const; - seastar::future<> omap_get_vals_by_keys( + ll_read_errorator::future<> omap_get_vals_by_keys( const ObjectState& os, OSDOp& osd_op) const; seastar::future<> omap_set_vals( diff --git a/src/crimson/osd/pg_meta.cc b/src/crimson/osd/pg_meta.cc index e5dda965aa66..ad538596355c 100644 --- a/src/crimson/osd/pg_meta.cc +++ b/src/crimson/osd/pg_meta.cc @@ -32,13 +32,14 @@ namespace { return std::make_optional(std::move(value)); } } + seastar::future PGMeta::get_epoch() { return store->open_collection(coll_t{pgid}).then([this](auto ch) { return store->omap_get_values(ch, pgid.make_pgmeta_oid(), {string{infover_key}, - string{epoch_key}}).then( + string{epoch_key}}).safe_then( [](auto&& values) { { // sanity check @@ -53,6 +54,9 @@ seastar::future PGMeta::get_epoch() assert(epoch); return seastar::make_ready_future(*epoch); } + }, + FuturizedStore::read_errorator::assert_all{ + "PGMeta::get_epoch: unable to read pgmeta" }); }); } @@ -66,7 +70,7 @@ seastar::future> PGMeta::load() string{info_key}, string{biginfo_key}, string{fastinfo_key}}); - }).then([](auto&& values) { + }).safe_then([](auto&& values) { { // sanity check auto infover = find_value<__u8>(values, infover_key); @@ -97,5 +101,8 @@ seastar::future> PGMeta::load() } return seastar::make_ready_future>( std::make_tuple(std::move(info), std::move(past_intervals))); + }, + FuturizedStore::read_errorator::assert_all{ + "PGMeta::load: unable to read pgmeta" }); } -- 2.47.3