From: Chunmei Liu <chunmei.liu@intel.com>
Date: Fri, 28 Feb 2020 01:50:15 +0000 (-0800)
Subject: crimson: complete context delete in alien world
X-Git-Tag: v15.1.1~182^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=5c90bafd731c5a5063bb6fef4bded322282c2cb8;p=ceph-ci.git

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 e3d176c314a..aa958aa140d 100644
--- 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 0269c133957..b08859a4319 100644
--- 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 26533b54499..6d39be55ba1 100644
--- 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 769f6b5e735..88c5fac6c7a 100644
--- 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);