From a7e44dbe151f7cc21c148abde2ab8149406c8f88 Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Sun, 25 Aug 2019 16:46:02 +0200 Subject: [PATCH] crimson: bring crimson::errorator with its first appliance. Signed-off-by: Radoslaw Zarzynski --- src/crimson/common/errorator.h | 185 +++++++++++++++++++++++++++++++ src/crimson/os/cyan_store.cc | 18 +-- src/crimson/os/cyan_store.h | 8 +- src/crimson/os/futurized_store.h | 7 +- src/crimson/osd/ops_executer.cc | 22 +++- src/crimson/osd/pg_backend.cc | 80 +++++++------ src/crimson/osd/pg_backend.h | 3 +- 7 files changed, 267 insertions(+), 56 deletions(-) create mode 100644 src/crimson/common/errorator.h diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h new file mode 100644 index 0000000000000..fb8214a04d5f2 --- /dev/null +++ b/src/crimson/common/errorator.h @@ -0,0 +1,185 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#pragma once + +#include + +namespace crimson { + +namespace _impl { + enum class ct_error { + enoent, + invarg, + enodata + }; +} + +// unthrowable_wrapper ensures compilation failure when somebody +// would like to `throw make_error<...>)()` instead of returning. +// returning allows for the compile-time verification of future's +// AllowedErrorsV and also avoid the burden of throwing. +template <_impl::ct_error ErrorV> struct unthrowable_wrapper { + unthrowable_wrapper(const unthrowable_wrapper&) = delete; + static constexpr unthrowable_wrapper instance{}; + template friend const T& make_error(); + + // comparison operator for the static_assert in future's ctor. + template <_impl::ct_error OtherErrorV> + constexpr bool operator==(const unthrowable_wrapper&) const { + return OtherErrorV == ErrorV; + } + +private: + // can be used only for initialize the `instance` member + explicit unthrowable_wrapper() = default; +}; + +template [[nodiscard]] const T& make_error() { + return T::instance; +} + +template +struct errorator { + template + class future : private seastar::future { + using base_t = seastar::future; + + // TODO: let `exception` use other type than `ct_error`. + template <_impl::ct_error V> + class exception { + exception() = default; + public: + exception(const exception&) = default; + }; + + template + class maybe_handle_error_t { + const std::type_info& type_info; + typename FuturatorT::type result; + ErrorVisitorT errfunc; + + public: + maybe_handle_error_t(ErrorVisitorT&& errfunc, std::exception_ptr ep) + : type_info(*ep.__cxa_exception_type()), + result(FuturatorT::make_exception_future(std::move(ep))), + errfunc(std::forward(errfunc)) { + } + + template <_impl::ct_error ErrorV> + void operator()(const unthrowable_wrapper& e) { + // In C++ throwing an exception isn't the sole way to signal + // error with it. This approach nicely fits cold, infrequent cases + // but when applied to a hot one, it will likely hurt performance. + // + // Alternative approach is to create `std::exception_ptr` on our + // own and place it in the future via `make_exception_future()`. + // When it comes to handling, the pointer can be interrogated for + // pointee's type with `__cxa_exception_type()` instead of costly + // re-throwing (via `std::rethrow_exception()`) and matching with + // `catch`. The limitation here is lack of support for hierarchies + // of exceptions. The code below checks for exact match only while + // `catch` would allow to match against a base class as well. + // However, this shouldn't be a big issue for `errorator` as Error + // Visitors are already checked for exhaustiveness at compile-time. + // + // NOTE: `__cxa_exception_type()` is an extension of the language. + // It should be available both in GCC and Clang but a fallback + // (based on `std::rethrow_exception()` and `catch`) can be made + // to handle other platforms is necessary. + if (type_info == typeid(exception)) { + // set `state::invalid` in internals of `seastar::future` to not + // `report_failed_future()` during `operator=()`. + std::move(result).get_exception(); + result = std::forward(errfunc)(e); + } + } + + auto get_result() && { + return std::move(result); + } + }; + + using base_t::base_t; + + [[gnu::always_inline]] + future(base_t&& base) + : base_t(std::move(base)) { + } + + public: + // initialize future as failed without throwing. `make_exception_future()` + // internally uses `std::make_exception_ptr()`. cppreference.com shouldn't + // be misinterpreted when it says: + // + // "This is done as if executing the following code: + // try { + // throw e; + // } catch(...) { + // return std::current_exception(); + // }", + // + // the "as if" is absolutely crucial because modern GCCs employ optimized + // path for it. See: + // * https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=cce8e59224e18858749a2324bce583bcfd160d6c, + // * https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00373.html. + // + // This behavior, combined with `__cxa_exception_type()` for inspecting + // exception's type, allows for throw/catch-free handling of stateless + // exceptions (which is fine for error codes). Stateful jumbos would be + // actually a bit harder as `_M_get()` is private, and thus rethrowing is + // necessary to get to the state inside. However, it's not unthinkable to + // see another extension bringing operator*() to the exception pointer... + // + // TODO: we don't really need to `make_exception_ptr` each time. It still + // allocates memory underneath while can be replaced with single instance + // per type created on start-up. + template <_impl::ct_error ErrorV> + future(const unthrowable_wrapper& e) + : base_t(seastar::make_exception_future(exception{})) { + // this is `fold expression` of C++17 + static_assert((... || (e == WrappedAllowedErrorsT::instance)), + "disallowed ct_error"); + } + + template + auto safe_then(ValueFuncT&& valfunc, ErrorVisitorT&& errfunc) { + return this->then_wrapped( + [ valfunc = std::forward(valfunc), + errfunc = std::forward(errfunc) + ] (auto future) mutable { + using futurator_t = \ + seastar::futurize>; + if (__builtin_expect(future.failed(), false)) { + maybe_handle_error_t maybe_handle_error( + std::forward(errfunc), + std::move(future).get_exception() + ); + (maybe_handle_error(WrappedAllowedErrorsT::instance) , ...); + return std::move(maybe_handle_error).get_result(); + } else { + return futurator_t::apply(std::forward(valfunc), + std::move(future).get()); + } + }); + } + + template + void then(Func&&) = delete; + + friend errorator; + }; + + template + static future its_error_free(seastar::future&& plain_future) { + return future(std::move(plain_future)); + } +}; // class errorator + +namespace ct_error { + using enoent = unthrowable_wrapper<_impl::ct_error::enoent>; + using enodata = unthrowable_wrapper<_impl::ct_error::enodata>; + using invarg = unthrowable_wrapper<_impl::ct_error::invarg>; +} + +} // namespace crimson diff --git a/src/crimson/os/cyan_store.cc b/src/crimson/os/cyan_store.cc index 60573ee172383..0dafec55cef72 100644 --- a/src/crimson/os/cyan_store.cc +++ b/src/crimson/os/cyan_store.cc @@ -198,23 +198,25 @@ seastar::future CyanStore::read(CollectionRef ch, return seastar::make_ready_future(o->read(offset, l)); } -seastar::future CyanStore::get_attr(CollectionRef ch, - const ghobject_t& oid, - std::string_view name) const +crimson::errorator::future +CyanStore::get_attr(CollectionRef ch, + const ghobject_t& oid, + std::string_view name) const { auto c = static_cast(ch.get()); logger().debug("{} {} {}", __func__, c->get_cid(), oid); auto o = c->get_object(oid); if (!o) { - return seastar::make_exception_future( - EnoentException(fmt::format("object does not exist: {}", oid))); + return crimson::make_error(); } if (auto found = o->xattr.find(name); found != o->xattr.end()) { - return seastar::make_ready_future(found->second); + return crimson::errorator::its_error_free( + seastar::make_ready_future(found->second)); } else { - return seastar::make_exception_future( - EnodataException(fmt::format("attr does not exist: {}/{}", oid, name))); + return crimson::make_error(); } } diff --git a/src/crimson/os/cyan_store.h b/src/crimson/os/cyan_store.h index 83a816d7a7cac..84311882e52b9 100644 --- a/src/crimson/os/cyan_store.h +++ b/src/crimson/os/cyan_store.h @@ -49,9 +49,11 @@ public: uint64_t offset, size_t len, uint32_t op_flags = 0) final; - seastar::future get_attr(CollectionRef c, - const ghobject_t& oid, - std::string_view name) const final; + virtual crimson::errorator::future + get_attr(CollectionRef c, + const ghobject_t& oid, + std::string_view name) const final; seastar::future get_attrs(CollectionRef c, const ghobject_t& oid) final; diff --git a/src/crimson/os/futurized_store.h b/src/crimson/os/futurized_store.h index 96415653bbfc8..afb448b050666 100644 --- a/src/crimson/os/futurized_store.h +++ b/src/crimson/os/futurized_store.h @@ -11,6 +11,7 @@ #include +#include "crimson/osd/exceptions.h" #include "include/buffer_fwd.h" #include "include/uuid.h" #include "osd/osd_types.h" @@ -76,9 +77,9 @@ public: uint64_t offset, size_t len, uint32_t op_flags = 0) = 0; - virtual seastar::future get_attr(CollectionRef c, - const ghobject_t& oid, - std::string_view name) const = 0; + virtual crimson::errorator::future + get_attr(CollectionRef c, const ghobject_t& oid, std::string_view name) const = 0; using attrs_t = std::map>; virtual seastar::future get_attrs(CollectionRef c, diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc index 0e527854e5156..385fcb0a8b959 100644 --- a/src/crimson/osd/ops_executer.cc +++ b/src/crimson/osd/ops_executer.cc @@ -167,19 +167,29 @@ static seastar::future pgls_filter( if (const auto xattr = filter.get_xattr(); !xattr.empty()) { logger().debug("pgls_filter: filter is interested in xattr={} for obj={}", xattr, sobj); - return backend.getxattr(sobj, xattr).then_wrapped( - [&filter, sobj] (auto futval) { + return backend.getxattr(sobj, xattr).safe_then( + [&filter, sobj] (ceph::bufferptr bp) { logger().debug("pgls_filter: got xvalue for obj={}", sobj); ceph::bufferlist val; - if (!futval.failed()) { - val.push_back(std::move(futval).get0()); - } else if (filter.reject_empty_xattr()) { + val.push_back(std::move(bp)); + const bool filtered = filter.filter(sobj, val); + return seastar::make_ready_future(filtered ? sobj : hobject_t{}); + }, [&filter, sobj] (const auto& e) { + // TODO: sugar-coat error handling. Compile-time visitors require + // too much of fragile boilerplate. + using T = std::decay_t; + static_assert(std::is_same_v || + std::is_same_v); + logger().debug("pgls_filter: got enoent for obj={}", sobj); + + if (filter.reject_empty_xattr()) { return seastar::make_ready_future(hobject_t{}); } + ceph::bufferlist val; const bool filtered = filter.filter(sobj, val); return seastar::make_ready_future(filtered ? sobj : hobject_t{}); - }); + }); } else { ceph::bufferlist empty_lvalue_bl; const bool filtered = filter.filter(sobj, empty_lvalue_bl); diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc index 4f7b3c88772a1..be8a2f8cfdad5 100644 --- a/src/crimson/osd/pg_backend.cc +++ b/src/crimson/osd/pg_backend.cc @@ -115,24 +115,27 @@ PGBackend::_load_os(const hobject_t& oid) } return store->get_attr(coll, ghobject_t{oid, ghobject_t::NO_GEN, shard}, - OI_ATTR).then_wrapped([oid, this](auto fut) { - if (fut.failed()) { - auto ep = std::move(fut).get_exception(); - if (!crimson::os::FuturizedStore::EnoentException::is_class_of(ep)) { - std::rethrow_exception(ep); - } - return seastar::make_ready_future( - os_cache.insert(oid, - std::make_unique(object_info_t{oid}, false))); - } else { + OI_ATTR) + .safe_then( + [oid, this] (ceph::bufferptr bp) { // decode existing OI_ATTR's value ceph::bufferlist bl; - bl.push_back(std::move(fut).get0()); + bl.push_back(std::move(bp)); return seastar::make_ready_future( os_cache.insert(oid, std::make_unique(object_info_t{bl}, true /* exists */))); - } - }); + }, + [oid, this] (const auto& e) { + using T = std::decay_t; + if constexpr (std::is_same_v || + std::is_same_v) { + return seastar::make_ready_future( + os_cache.insert(oid, + std::make_unique(object_info_t{oid}, false))); + } else { + static_assert(always_false::value, "non-exhaustive visitor!"); + } + }); } seastar::future @@ -143,24 +146,25 @@ PGBackend::_load_ss(const hobject_t& oid) } return store->get_attr(coll, ghobject_t{oid, ghobject_t::NO_GEN, shard}, - SS_ATTR).then_wrapped([oid, this](auto fut) { - std::unique_ptr snapset; - if (fut.failed()) { - auto ep = std::move(fut).get_exception(); - if (!crimson::os::FuturizedStore::EnoentException::is_class_of(ep)) { - std::rethrow_exception(ep); - } else { - snapset = std::make_unique(); - } - } else { + SS_ATTR) + .safe_then( + [oid, this] (ceph::bufferptr bp) { // decode existing SS_ATTR's value ceph::bufferlist bl; - bl.push_back(std::move(fut).get0()); - snapset = std::make_unique(bl); - } - return seastar::make_ready_future( - ss_cache.insert(oid, std::move(snapset))); - }); + bl.push_back(std::move(bp)); + return seastar::make_ready_future( + ss_cache.insert(oid, std::make_unique(bl))); + }, + [oid, this] (const auto& e) { + using T = std::decay_t; + if constexpr (std::is_same_v || + std::is_same_v) { + return seastar::make_ready_future( + ss_cache.insert(oid, std::make_unique())); + } else { + static_assert(always_false::value, "non-exhaustive visitor!"); + } + }); } seastar::future @@ -475,22 +479,28 @@ seastar::future<> PGBackend::getxattr( name = "_" + aname; } logger().debug("getxattr on obj={} for attr={}", os.oi.soid, name); - return getxattr(os.oi.soid, name).then([&osd_op] (ceph::bufferptr val) { + return getxattr(os.oi.soid, name).safe_then([&osd_op] (ceph::bufferptr val) { osd_op.outdata.clear(); osd_op.outdata.push_back(std::move(val)); osd_op.op.xattr.value_len = osd_op.outdata.length(); + return seastar::now(); //ctx->delta_stats.num_rd_kb += shift_round_up(osd_op.outdata.length(), 10); - }).handle_exception_type( - [] (crimson::os::FuturizedStore::EnoentException&) { + }, [] (const auto& e) { + using T = std::decay_t; + if constexpr (std::is_same_v) { return seastar::make_exception_future<>(crimson::osd::object_not_found{}); - }).handle_exception_type( - [] (crimson::os::FuturizedStore::EnodataException&) { + } else if constexpr (std::is_same_v) { return seastar::make_exception_future<>(crimson::osd::no_message_available{}); + } else { + static_assert(always_false::value, "non-exhaustive visitor!"); + } }); //ctx->delta_stats.num_rd++; } -seastar::future PGBackend::getxattr( +crimson::errorator::future +PGBackend::getxattr( const hobject_t& soid, std::string_view key) const { diff --git a/src/crimson/osd/pg_backend.h b/src/crimson/osd/pg_backend.h index b4a7b74a5df44..e831fcfe9c7f5 100644 --- a/src/crimson/osd/pg_backend.h +++ b/src/crimson/osd/pg_backend.h @@ -87,7 +87,8 @@ public: seastar::future<> getxattr( const ObjectState& os, OSDOp& osd_op) const; - seastar::future getxattr( + crimson::errorator::future getxattr( const hobject_t& soid, std::string_view key) const; -- 2.39.5