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 <sjust@redhat.com>

diff --git a/src/crimson/os/alienstore/alien_store.cc b/src/crimson/os/alienstore/alien_store.cc
index 3df7a65677645009f3913f9a89e8db691ffd8371..ace930a110bcc4e956e0fb62cb674fc496465a3f 100644 (file)
--- 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<AlienCollection*>(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<AlienCollection*>(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 d33941aee0c2f69500cd6e54750f3d149b482581..db08dc133d3c64262b0eebb353b3982976d1e48f 100644 (file)
--- a/src/crimson/os/alienstore/alien_store.h
+++ b/src/crimson/os/alienstore/alien_store.h
@@ -89,8 +89,9 @@ public:
   seastar::future<CollectionRef> open_collection(const coll_t& cid) final;
   seastar::future<std::vector<coll_t>> 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 0126ce02f1298c490a53955aca60f49cc3b2b52e..451049acd4efde748cca74192343f05e9af95b1b 100644 (file)
--- 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 ab0839eecfee75615774bdf2bbf44bc5e9f9c176..7af41aa6fe2507c1fc74a5b5bc240cd99bbe9ebc 100644 (file)
--- a/src/crimson/os/cyanstore/cyan_store.h
+++ b/src/crimson/os/cyanstore/cyan_store.h
@@ -121,8 +121,9 @@ public:
   seastar::future<CollectionRef> open_collection(const coll_t& cid) final;
   seastar::future<std::vector<coll_t>> 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 0122625fba055d2151a87818bdd55daca711ddad..0e4e8c6980aa722a5212cdf116aa7f1da8a826b4 100644 (file)
--- a/src/crimson/os/futurized_store.h
+++ b/src/crimson/os/futurized_store.h
@@ -134,8 +134,27 @@ public:
   virtual seastar::future<CollectionRef> open_collection(const coll_t& cid) = 0;
   virtual seastar::future<std::vector<coll_t>> 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<Context> 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<std::vector<coll_t>> 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 079b17713f767671a78be63150dfe3b269331ac8..ec8752512915578c138f132be9cbf78b7af0ea76 100644 (file)
--- 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 4163deafe8823965fde2d91610c8698332c8137b..fbfb03ebeb42b9339956b909db172add7605f077 100644 (file)
--- a/src/crimson/os/seastore/seastore.h
+++ b/src/crimson/os/seastore/seastore.h
@@ -147,7 +147,7 @@ public:
   seastar::future<CollectionRef> open_collection(const coll_t& cid) final;
   seastar::future<std::vector<coll_t>> list_collections() final;
 
-  seastar::future<> do_transaction(
+  seastar::future<> do_transaction_no_callbacks(
     CollectionRef ch,
     ceph::os::Transaction&& txn) final;