crimson: complete context delete in alien world

This way, we allocate and release only the OnCommit Context on the bluestore
side.  Similarly, the crimson_wrapper context containing the passed in
contexts is allocated and released only on the crimson side.

Further, update do_transaction to also call on_applied and on_applied_sync
for completeness.

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 e3d176c314a98c592123973aa84ab2410bbcb7ec..aa958aa140d6267c8e4a0a812f248fb4f6940192 100644 (file)
--- a/src/crimson/os/alienstore/alien_store.cc
+++ b/src/crimson/os/alienstore/alien_store.cc
@@ -33,24 +33,23 @@ namespace {
 class OnCommit final: public Context
 {
   int cpuid;
-  Context* on_commit = nullptr;
+  Context *oncommit;
+  seastar::promise<> &alien_done;
 public:
-  seastar::promise<> alien_done;
-  OnCommit(int id, ceph::os::Transaction& txn): cpuid(id) {
-    if (txn.has_contexts()) {
-      on_commit = txn.get_on_commit();
-    }
-  }
+  OnCommit(
+    int id,
+    seastar::promise<> &done,
+    Context *oncommit,
+    ceph::os::Transaction& txn)
+    : cpuid(id), oncommit(oncommit),
+      alien_done(done) {}
 
   void finish(int) final {
-    auto fut = seastar::alien::submit_to(cpuid, [this] {
-      if (on_commit) {
-        on_commit->complete(0);
-      }
+    return seastar::alien::submit_to(cpuid, [this] {
+      if (oncommit) oncommit->complete(0);
       alien_done.set_value();
       return seastar::make_ready_future<>();
-    });
-    fut.wait();
+    }).wait();
   }
 };
 }
@@ -304,23 +303,27 @@ seastar::future<> AlienStore::do_transaction(CollectionRef ch,
                                              ceph::os::Transaction&& txn)
 {
   logger().debug("{}", __func__);
-  auto callback =
-    std::make_unique<OnCommit>(seastar::engine().cpu_id(), txn);
-  return seastar::do_with(std::move(txn), std::move(callback),
-      [this, ch] (ceph::os::Transaction &txn, auto &callback) {
-    return seastar::with_gate(transaction_gate, [this, ch, &txn, &callback] {
-      return tp_mutex.lock().then ([this, ch, &txn, &callback] {
-        return tp->submit([=, &txn, &callback] {
-          txn.register_on_commit(callback.get());
-          auto c = static_cast<AlienCollection*>(ch.get());
-          return store->queue_transaction(c->collection, std::move(txn));
-        });
-      }).then([this, &callback] (int) {
-       tp_mutex.unlock();
-        return callback->alien_done.get_future();
+  auto id = seastar::engine().cpu_id();
+  auto done = seastar::promise<>();
+  return seastar::do_with(
+    std::move(txn),
+    std::move(done),
+    [this, ch, id] (auto &txn, auto &done) {
+      return seastar::with_gate(transaction_gate, [this, ch, id, &txn, &done] {
+       return tp_mutex.lock().then ([this, ch, id, &txn, &done] {
+         Context *crimson_wrapper =
+           ceph::os::Transaction::collect_all_contexts(txn);
+         return tp->submit([this, ch, id, crimson_wrapper, &txn, &done] {
+           txn.register_on_commit(new OnCommit(id, done, crimson_wrapper, txn));
+           auto c = static_cast<AlienCollection*>(ch.get());
+           return store->queue_transaction(c->collection, std::move(txn));
+         });
+       }).then([this, &done] (int) {
+         tp_mutex.unlock();
+         return done.get_future();
+       });
       });
     });
-  });
 }
 
 seastar::future<> AlienStore::write_meta(const std::string& key,

diff --git a/src/crimson/os/cyanstore/cyan_store.cc b/src/crimson/os/cyanstore/cyan_store.cc
index 0269c133957e35145cc74949c79dbec7dcde9c08..b08859a43196de8c109a83312fc2362e50ac0cd5 100644 (file)
--- a/src/crimson/os/cyanstore/cyan_store.cc
+++ b/src/crimson/os/cyanstore/cyan_store.cc
@@ -416,7 +416,6 @@ seastar::future<> CyanStore::do_transaction(CollectionRef ch,
       t.get_on_applied_sync()}) {
     if (i) {
       i->complete(0);
-      delete i;
     }
   }
   return seastar::now();

diff --git a/src/include/Context.h b/src/include/Context.h
index 26533b544993224e20885840b6188ff3b6d44311..6d39be55ba1a25fa0703a98fb6282a1c939c5746 100644 (file)
--- a/src/include/Context.h
+++ b/src/include/Context.h
@@ -75,9 +75,7 @@ class Context {
   virtual ~Context() {}       // we want a virtual destructor!!!
   virtual void complete(int r) {
     finish(r);
-#ifndef WITH_SEASTAR
-    delete this;   //alien store need its callback fun alive to get future.
-#endif
+    delete this;
   }
   virtual bool sync_complete(int r) {
     if (sync_finish(r)) {

diff --git a/src/os/Transaction.h b/src/os/Transaction.h
index 769f6b5e7350482663a2682fa0044b33ee626826..88c5fac6c7a484024f51ea122875c908f93e9d86 100644 (file)
--- a/src/os/Transaction.h
+++ b/src/os/Transaction.h
@@ -360,6 +360,14 @@ public:
                                    i.on_applied_sync);
     }
   }
+  static Context *collect_all_contexts(
+    Transaction& t) {
+    list<Context*> contexts;
+    contexts.splice(contexts.end(), t.on_applied);
+    contexts.splice(contexts.end(), t.on_commit);
+    contexts.splice(contexts.end(), t.on_applied_sync);
+    return C_Contexts::list_to_context(contexts);
+  }
 
   Context *get_on_applied() {
     return C_Contexts::list_to_context(on_applied);