From cc59c8248312e2d865a2f05bb8a801d4428e7cd4 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 29 May 2021 16:03:50 +0800 Subject: [PATCH] crimson/os/alienstore: create tp in AlienStore::start() thread pool is not needed until AlienStore::start(). with this change, we are able to tell if the AlienStore is actually started or not in AlienStore::stop(). as seastar::sharded start a service in two phases: 1. construct the shard instances 2. actually start them and it stops a service in a single shot, which both stops the services and destructs the service instance(s). so we have to implement a proper stop() method for services whose start() might not be called after its instance is created by seastar::sharded::start() in case of error handling or if we just don't want to call start(). to ensure we can skip the steps to clean up the stuff created by start(), we need to have a flag in the sharded service, because AlienStore is a member variable of OSD, and when we do mkfs, AlienStore is not start()'ed, and as explained above, we have to call OSD::stop() to ensure OSD instance is destructed properly. but OSD::stop() calls store->umount() and store->stop() unconditionally. these methods in AlienStore rely on a functional thread pool. fortunately, we don't need to call these methods if the store is never mounted or started. in a case of failed "mkfs", store is not mounted at all but the store and osd instances are created. so, in this change, thread pool is created in AlienStore::start(), and we will use it to tell if AlienStore is started or not in the following change which makes the related method no-op if AlienStore is not started yet. also, postpone the creation of `store` until in AlienStore::start(), so we don't need to destroy it in the dtor of AlienStore. otherwise, BlueStore::~BlueStore() would need to reference resources which are only available in alien threads, but when OSD::~OSD() is called, we are in seastar's reactor. Signed-off-by: Kefu Chai --- src/crimson/os/alienstore/alien_store.cc | 36 +++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc index ced278ad2f2..c2e9f1a669d 100644 --- a/src/crimson/os/alienstore/alien_store.cc +++ b/src/crimson/os/alienstore/alien_store.cc @@ -67,6 +67,10 @@ AlienStore::AlienStore(const std::string& path, const ConfigValues& values) cct = std::make_unique(CEPH_ENTITY_TYPE_OSD); g_ceph_context = cct.get(); cct->_conf.set_config_values(values); +} + +seastar::future<> AlienStore::start() +{ store = std::make_unique(cct.get(), path); std::vector cpu_cores = _parse_cpu_cores(); // cores except the first "N_CORES_FOR_SEASTAR" ones will @@ -86,15 +90,12 @@ AlienStore::AlienStore(const std::string& path, const ConfigValues& values) const auto num_threads = get_conf("crimson_alien_op_num_threads"); tp = std::make_unique(num_threads, 128, cpu_cores); -} - -seastar::future<> AlienStore::start() -{ return tp->start(); } seastar::future<> AlienStore::stop() { + assert(tp); return tp->submit([this] { for (auto [cid, ch]: coll_map) { static_cast(ch.get())->collection.reset(); @@ -110,6 +111,7 @@ AlienStore::~AlienStore() = default; seastar::future<> AlienStore::mount() { logger().debug("{}", __func__); + assert(tp); return tp->submit([this] { return store->mount(); }).then([] (int r) { @@ -121,6 +123,7 @@ seastar::future<> AlienStore::mount() seastar::future<> AlienStore::umount() { logger().info("{}", __func__); + assert(tp); return transaction_gate.close().then([this] { return tp->submit([this] { return store->umount(); @@ -135,6 +138,7 @@ seastar::future<> AlienStore::mkfs(uuid_d osd_fsid) { logger().debug("{}", __func__); store->set_fsid(osd_fsid); + assert(tp); return tp->submit([this] { return store->mkfs(); }).then([] (int r) { @@ -150,6 +154,7 @@ AlienStore::list_objects(CollectionRef ch, uint64_t limit) const { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(std::vector(), ghobject_t(), [=] (auto &objects, auto &next) { objects.reserve(limit); @@ -171,6 +176,7 @@ AlienStore::list_objects(CollectionRef ch, seastar::future AlienStore::create_new_collection(const coll_t& cid) { logger().debug("{}", __func__); + assert(tp); return tp->submit([this, cid] { return store->create_new_collection(cid); }).then([this, cid] (ObjectStore::CollectionHandle c) { @@ -194,6 +200,7 @@ seastar::future AlienStore::create_new_collection(const coll_t& c seastar::future AlienStore::open_collection(const coll_t& cid) { logger().debug("{}", __func__); + assert(tp); return tp->submit([this, cid] { return store->open_collection(cid); }).then([this] (ObjectStore::CollectionHandle c) { @@ -216,6 +223,7 @@ seastar::future AlienStore::open_collection(const coll_t& cid) seastar::future> AlienStore::list_collections() { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(std::vector{}, [=] (auto &ls) { return tp->submit([this, &ls] { @@ -235,6 +243,7 @@ AlienStore::read(CollectionRef ch, uint32_t op_flags) { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(ceph::bufferlist{}, [=] (auto &bl) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &bl] { auto c = static_cast(ch.get()); @@ -259,6 +268,7 @@ AlienStore::readv(CollectionRef ch, uint32_t op_flags) { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(ceph::bufferlist{}, [this, ch, oid, &m, op_flags](auto& bl) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), @@ -284,6 +294,7 @@ AlienStore::get_attr(CollectionRef ch, std::string_view name) const { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(ceph::bufferlist{}, [=] (auto &value) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &value] { auto c =static_cast(ch.get()); @@ -307,6 +318,7 @@ AlienStore::get_attrs(CollectionRef ch, const ghobject_t& oid) { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(attrs_t{}, [=] (auto &aset) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &aset] { auto c = static_cast(ch.get()); @@ -328,6 +340,7 @@ auto AlienStore::omap_get_values(CollectionRef ch, -> read_errorator::future { logger().debug("{}", __func__); + assert(tp); return seastar::do_with(omap_values_t{}, [=] (auto &values) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &values] { auto c = static_cast(ch.get()); @@ -351,6 +364,7 @@ auto AlienStore::omap_get_values(CollectionRef ch, -> read_errorator::future> { logger().debug("{} with_start", __func__); + assert(tp); return seastar::do_with(omap_values_t{}, [=] (auto &values) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &values] { auto c = static_cast(ch.get()); @@ -386,6 +400,7 @@ seastar::future<> AlienStore::do_transaction(CollectionRef ch, return alien_coll->with_lock([this, 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] { txn.register_on_commit(new OnCommit(id, done, crimson_wrapper, txn)); @@ -403,6 +418,7 @@ seastar::future<> AlienStore::do_transaction(CollectionRef ch, seastar::future<> AlienStore::inject_data_error(const ghobject_t& o) { logger().debug("{}", __func__); + assert(tp); return tp->submit([=] { return store->inject_data_error(o); }); @@ -411,6 +427,7 @@ seastar::future<> AlienStore::inject_data_error(const ghobject_t& o) seastar::future<> AlienStore::inject_mdata_error(const ghobject_t& o) { logger().debug("{}", __func__); + assert(tp); return tp->submit([=] { return store->inject_mdata_error(o); }); @@ -420,6 +437,7 @@ seastar::future<> AlienStore::write_meta(const std::string& key, const std::string& value) { logger().debug("{}", __func__); + assert(tp); return tp->submit([=] { return store->write_meta(key, value); }).then([] (int r) { @@ -432,6 +450,7 @@ seastar::future> AlienStore::read_meta(const std::string& key) { logger().debug("{}", __func__); + assert(tp); return tp->submit([this, key] { std::string value; int r = store->read_meta(key, &value); @@ -458,6 +477,7 @@ uuid_d AlienStore::get_fsid() const seastar::future AlienStore::stat() const { logger().info("{}", __func__); + assert(tp); return seastar::do_with(store_statfs_t{}, [this] (store_statfs_t &st) { return tp->submit([this, &st] { return store->statfs(&st, nullptr); @@ -478,6 +498,7 @@ seastar::future AlienStore::stat( CollectionRef ch, const ghobject_t& oid) { + assert(tp); return seastar::do_with((struct stat){}, [this, ch, oid](auto& st) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [this, ch, oid, &st] { auto c = static_cast(ch.get()); @@ -491,6 +512,7 @@ auto AlienStore::omap_get_header(CollectionRef ch, const ghobject_t& oid) -> read_errorator::future { + assert(tp); return seastar::do_with(ceph::bufferlist(), [=](auto& bl) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &bl] { auto c = static_cast(ch.get()); @@ -515,6 +537,7 @@ seastar::future> AlienStore::fiemap( uint64_t off, uint64_t len) { + assert(tp); return seastar::do_with(std::map(), [=](auto& destmap) { return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &destmap] { auto c = static_cast(ch.get()); @@ -530,6 +553,7 @@ seastar::future AlienStore::get_omap_iterator( CollectionRef ch, const ghobject_t& oid) { + assert(tp); return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [this, ch, oid] { auto c = static_cast(ch.get()); @@ -545,6 +569,7 @@ seastar::future AlienStore::get_omap_iterator( // needs further optimization. seastar::future<> AlienStore::AlienOmapIterator::seek_to_first() { + assert(store->tp); return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()), [this] { return iter->seek_to_first(); @@ -557,6 +582,7 @@ seastar::future<> AlienStore::AlienOmapIterator::seek_to_first() seastar::future<> AlienStore::AlienOmapIterator::upper_bound( const std::string& after) { + assert(store->tp); return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()), [this, after] { return iter->upper_bound(after); @@ -569,6 +595,7 @@ seastar::future<> AlienStore::AlienOmapIterator::upper_bound( seastar::future<> AlienStore::AlienOmapIterator::lower_bound( const std::string& to) { + assert(store->tp); return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()), [this, to] { return iter->lower_bound(to); @@ -580,6 +607,7 @@ seastar::future<> AlienStore::AlienOmapIterator::lower_bound( seastar::future<> AlienStore::AlienOmapIterator::next() { + assert(store->tp); return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()), [this] { return iter->next(); -- 2.39.5