From 2b5c471913990fd53d9b5d4fff3445cbd009dafd Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 12 Sep 2022 22:14:16 -0700 Subject: [PATCH] crimson/os: consolidate context handling in FuturizedStore Context handling is pretty uniform accross all implementations, may as well do it in the same place. ShardedStoreProxy would need to handle it otherwise, since callbacks need to be handled on the core do_transaction is invoked on. Signed-off-by: Samuel Just --- src/crimson/os/alienstore/alien_store.cc | 18 +++++----------- src/crimson/os/alienstore/alien_store.h | 5 +++-- src/crimson/os/cyanstore/cyan_store.cc | 13 +++--------- src/crimson/os/cyanstore/cyan_store.h | 5 +++-- src/crimson/os/futurized_store.h | 27 ++++++++++++++++++++---- src/crimson/os/seastore/seastore.cc | 12 +---------- src/crimson/os/seastore/seastore.h | 2 +- 7 files changed, 39 insertions(+), 43 deletions(-) diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc index 3df7a656776..ace930a110b 100644 --- a/src/crimson/os/alienstore/alien_store.cc +++ b/src/crimson/os/alienstore/alien_store.cc @@ -39,27 +39,21 @@ seastar::logger& logger() class OnCommit final: public Context { const int cpuid; - Context *oncommit; seastar::alien::instance &alien; seastar::promise<> &alien_done; public: OnCommit( int id, seastar::promise<> &done, - Context *oncommit, seastar::alien::instance &alien, ceph::os::Transaction& txn) : cpuid(id), - oncommit(oncommit), alien(alien), alien_done(done) { } void finish(int) final { return seastar::alien::submit_to(alien, cpuid, [this] { - if (oncommit) { - oncommit->complete(0); - } alien_done.set_value(); return seastar::make_ready_future<>(); }).wait(); @@ -437,8 +431,9 @@ auto AlienStore::omap_get_values(CollectionRef ch, }); } -seastar::future<> AlienStore::do_transaction(CollectionRef ch, - ceph::os::Transaction&& txn) +seastar::future<> AlienStore::do_transaction_no_callbacks( + CollectionRef ch, + ceph::os::Transaction&& txn) { logger().debug("{}", __func__); auto id = seastar::this_shard_id(); @@ -450,13 +445,10 @@ seastar::future<> AlienStore::do_transaction(CollectionRef ch, AlienCollection* alien_coll = static_cast(ch.get()); // moving the `ch` is crucial for buildability on newer S* versions. return alien_coll->with_lock([this, ch=std::move(ch), id, &txn, &done] { - Context *crimson_wrapper = - ceph::os::Transaction::collect_all_contexts(txn); assert(tp); return tp->submit(ch->get_cid().hash_to_shard(tp->size()), - [this, ch, id, crimson_wrapper, &txn, &done, &alien=seastar::engine().alien()] { - txn.register_on_commit(new OnCommit(id, done, crimson_wrapper, - alien, txn)); + [this, ch, id, &txn, &done, &alien=seastar::engine().alien()] { + txn.register_on_commit(new OnCommit(id, done, alien, txn)); auto c = static_cast(ch.get()); return store->queue_transaction(c->collection, std::move(txn)); }); diff --git a/src/crimson/os/alienstore/alien_store.h b/src/crimson/os/alienstore/alien_store.h index d33941aee0c..db08dc133d3 100644 --- a/src/crimson/os/alienstore/alien_store.h +++ b/src/crimson/os/alienstore/alien_store.h @@ -89,8 +89,9 @@ public: seastar::future open_collection(const coll_t& cid) final; seastar::future> list_collections() final; - seastar::future<> do_transaction(CollectionRef c, - ceph::os::Transaction&& txn) final; + seastar::future<> do_transaction_no_callbacks( + CollectionRef c, + ceph::os::Transaction&& txn) final; // error injection seastar::future<> inject_data_error(const ghobject_t& o) final; diff --git a/src/crimson/os/cyanstore/cyan_store.cc b/src/crimson/os/cyanstore/cyan_store.cc index 0126ce02f12..451049acd4e 100644 --- a/src/crimson/os/cyanstore/cyan_store.cc +++ b/src/crimson/os/cyanstore/cyan_store.cc @@ -334,8 +334,9 @@ CyanStore::omap_get_header(CollectionRef ch, o->omap_header); } -seastar::future<> CyanStore::do_transaction(CollectionRef ch, - ceph::os::Transaction&& t) +seastar::future<> CyanStore::do_transaction_no_callbacks( + CollectionRef ch, + ceph::os::Transaction&& t) { using ceph::os::Transaction; int r = 0; @@ -522,14 +523,6 @@ seastar::future<> CyanStore::do_transaction(CollectionRef ch, logger().error("{}", str.str()); ceph_assert(r == 0); } - for (auto i : { - t.get_on_applied(), - t.get_on_commit(), - t.get_on_applied_sync()}) { - if (i) { - i->complete(0); - } - } return seastar::now(); } diff --git a/src/crimson/os/cyanstore/cyan_store.h b/src/crimson/os/cyanstore/cyan_store.h index ab0839eecfe..7af41aa6fe2 100644 --- a/src/crimson/os/cyanstore/cyan_store.h +++ b/src/crimson/os/cyanstore/cyan_store.h @@ -121,8 +121,9 @@ public: seastar::future open_collection(const coll_t& cid) final; seastar::future> list_collections() final; - seastar::future<> do_transaction(CollectionRef ch, - ceph::os::Transaction&& txn) final; + seastar::future<> do_transaction_no_callbacks( + CollectionRef ch, + ceph::os::Transaction&& txn) final; seastar::future<> write_meta(const std::string& key, const std::string& value) final; diff --git a/src/crimson/os/futurized_store.h b/src/crimson/os/futurized_store.h index 0122625fba0..0e4e8c6980a 100644 --- a/src/crimson/os/futurized_store.h +++ b/src/crimson/os/futurized_store.h @@ -134,8 +134,27 @@ public: virtual seastar::future open_collection(const coll_t& cid) = 0; virtual seastar::future> list_collections() = 0; - virtual seastar::future<> do_transaction(CollectionRef ch, - ceph::os::Transaction&& txn) = 0; +protected: + virtual seastar::future<> do_transaction_no_callbacks( + CollectionRef ch, + ceph::os::Transaction&& txn) = 0; + +public: + seastar::future<> do_transaction( + CollectionRef ch, + ceph::os::Transaction&& txn) { + std::unique_ptr on_commit( + ceph::os::Transaction::collect_all_contexts(txn)); + return do_transaction_no_callbacks( + std::move(ch), std::move(txn) + ).then([on_commit=std::move(on_commit)]() mutable { + auto c = on_commit.release(); + if (c) c->complete(0); + return seastar::now(); + }); + } + + /** * flush * @@ -378,10 +397,10 @@ public: seastar::future> list_collections() final { return proxy(&T::list_collections); } - seastar::future<> do_transaction( + seastar::future<> do_transaction_no_callbacks( CollectionRef ch, ceph::os::Transaction &&txn) final { - return proxy(&T::do_transaction, std::move(ch), std::move(txn)); + return proxy(&T::do_transaction_no_callbacks, std::move(ch), std::move(txn)); } seastar::future<> flush(CollectionRef ch) final { return proxy(&T::flush, std::move(ch)); diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index 079b17713f7..ec875251291 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -1104,7 +1104,7 @@ void SeaStore::on_error(ceph::os::Transaction &t) { abort(); } -seastar::future<> SeaStore::do_transaction( +seastar::future<> SeaStore::do_transaction_no_callbacks( CollectionRef _ch, ceph::os::Transaction&& _t) { @@ -1141,16 +1141,6 @@ seastar::future<> SeaStore::do_transaction( }).si_then([this, &ctx] { return transaction_manager->submit_transaction(*ctx.transaction); }); - }).safe_then([&ctx]() { - for (auto i : { - ctx.ext_transaction.get_on_applied(), - ctx.ext_transaction.get_on_commit(), - ctx.ext_transaction.get_on_applied_sync()}) { - if (i) { - i->complete(0); - } - } - return seastar::now(); }); }); } diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h index 4163deafe88..fbfb03ebeb4 100644 --- a/src/crimson/os/seastore/seastore.h +++ b/src/crimson/os/seastore/seastore.h @@ -147,7 +147,7 @@ public: seastar::future open_collection(const coll_t& cid) final; seastar::future> list_collections() final; - seastar::future<> do_transaction( + seastar::future<> do_transaction_no_callbacks( CollectionRef ch, ceph::os::Transaction&& txn) final; -- 2.39.5