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<Service> 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<Service>::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 <kchai@redhat.com>

diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc
index ced278ad2f2f2dfa9b09cffa5354c95a256e0356..c2e9f1a669dc7505597650740941ea9e7d797e79 100644 (file)
--- 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<CephContext>(CEPH_ENTITY_TYPE_OSD);
   g_ceph_context = cct.get();
   cct->_conf.set_config_values(values);
+}
+
+seastar::future<> AlienStore::start()
+{
   store = std::make_unique<BlueStore>(cct.get(), path);
   std::vector<uint64_t> 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<uint64_t>("crimson_alien_op_num_threads");
   tp = std::make_unique<crimson::os::ThreadPool>(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<AlienCollection*>(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>(), ghobject_t(),
                           [=] (auto &objects, auto &next) {
     objects.reserve(limit);
@@ -171,6 +176,7 @@ AlienStore::list_objects(CollectionRef ch,
 seastar::future<CollectionRef> 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<CollectionRef> AlienStore::create_new_collection(const coll_t& c
 seastar::future<CollectionRef> 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<CollectionRef> AlienStore::open_collection(const coll_t& cid)
 seastar::future<std::vector<coll_t>> AlienStore::list_collections()
 {
   logger().debug("{}", __func__);
+  assert(tp);
 
   return seastar::do_with(std::vector<coll_t>{}, [=] (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<AlienCollection*>(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<AlienCollection*>(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<AlienCollection*>(ch.get());
@@ -328,6 +340,7 @@ auto AlienStore::omap_get_values(CollectionRef ch,
   -> read_errorator::future<omap_values_t>
 {
   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<AlienCollection*>(ch.get());
@@ -351,6 +364,7 @@ auto AlienStore::omap_get_values(CollectionRef ch,
   -> read_errorator::future<std::tuple<bool, omap_values_t>>
 {
   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<AlienCollection*>(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<std::tuple<int, std::string>>
 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<store_statfs_t> 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<struct stat> 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<AlienCollection*>(ch.get());
@@ -491,6 +512,7 @@ auto AlienStore::omap_get_header(CollectionRef ch,
                                  const ghobject_t& oid)
   -> read_errorator::future<ceph::bufferlist>
 {
+  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<AlienCollection*>(ch.get());
@@ -515,6 +537,7 @@ seastar::future<std::map<uint64_t, uint64_t>> AlienStore::fiemap(
   uint64_t off,
   uint64_t len)
 {
+  assert(tp);
   return seastar::do_with(std::map<uint64_t, uint64_t>(), [=](auto& destmap) {
     return tp->submit(ch->get_cid().hash_to_shard(tp->size()), [=, &destmap] {
       auto c = static_cast<AlienCollection*>(ch.get());
@@ -530,6 +553,7 @@ seastar::future<FuturizedStore::OmapIteratorRef> 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<AlienCollection*>(ch.get());
@@ -545,6 +569,7 @@ seastar::future<FuturizedStore::OmapIteratorRef> 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();