osdc: Extend op_post_submit to cope with successful Ops.

The locking situation in Objecter is complex. When ops are completed whether
with success or otherwise, some locks are held. For split ops, this is
particularly complex, since multiple sessions are involved in the completion.

To avoid all these deadlock issues, splitOps choose to schedule a completion
task using asio::post, which can then take the appropriate locks before
completing the IO, without risk of deadlock.

Usage of this will be added in a refactor if SplitOps.

Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 4e9057ca69ba8d6ac4e935abae453d11425129a0..db38fc11df28fac79876675e35fd432a8d51c906 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -2364,10 +2364,49 @@ void Objecter::resend_mon_ops()
 
 // read | write ---------------------------
 
+void Objecter::op_post_split_op_complete(Op* op, bs::error_code ec, int rc) {
 
-void Objecter::op_post_submit(Op* op) {
-  boost::asio::post(service, [this, op]() {
-    op_submit(op);
+  // If the parent op was already removed from its session by a map change, then
+  // ignore the completion here.
+  if (!op->session) {
+    return;
+  }
+
+  // While the following post is scheduled, this op is considered "in flight".
+  // If a new osdmap is published, this op might be cancelled/redriven, etc..
+  // before the following post is executed. Normally, an op would be protected
+  // from this in the messaging layer. However, we don't have that protection
+  // here.  So we consider the following:
+  // 1. The op could be returned and freed (op->get() prevents this).
+  // 2. The op could be removed from its session but not yet resent.
+  // 3. The op could be redriven in the new epoch.
+  op->get();
+  auto epoch = op->target.epoch;
+
+  boost::asio::post(service, [this, op, ec, rc, epoch]() {
+
+    bool freed = op->get_nref() == 1;
+    op->put();
+    if (freed || !op->session || op->target.epoch != epoch) {
+      return;
+    }
+
+    shunique_lock rl(rwlock, ceph::acquire_shared);
+    ceph_assert(op->session);
+    unique_lock sl(op->session->lock);
+
+    if (rc != -EAGAIN) {
+      op->trace.event("post op complete");
+
+      // This function unlocks sl.
+      complete_op_reply(op, ec, op->session, sl, rc);
+    } else {
+      _session_op_remove(op->session, op);
+      sl.unlock();
+      op->split_op_tids.reset();
+      ceph_tid_t tid = 0;
+      _op_submit(op, rl, &tid);
+    }
   });
 }
 

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 2620c3cbc4852067dfb59ab62d91d5c52fe77d65..f4fcd9c4b20cc7dea73a98c1874f3a9cf305aa37 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -2847,7 +2847,7 @@ private:
                              int *ctx_budget = NULL);
   // public interface
 public:
-  void op_post_submit(Op *op);
+  void op_post_split_op_complete(Op* op, boost::system::error_code ec, int rc);
   void op_submit(Op *op, ceph_tid_t *ptid = NULL, int *ctx_budget = NULL);
   bool is_active() {
     std::shared_lock l(rwlock);