osd/ReplicatedPG: carry CopyOpRef in copy_from completion

There is a race with copy_from cancellation.  The internal Objecter
completion decodes a bunch of data and copies it into pointers provided
when the op is queued.  When we cancel, we need to ensure that we can cope
until control passes back to our provided completion.

Once we *do* get into the (ReplicatedPG) callbacks, we will bail out
because the tid in the CopyOp or FlushOp no longer matches.

Fix this by carrying a ref to keep the copy-from targets alive, and
clearing out the tids that we cancel.

Note that previously, the trigger for this was that the tid changes when
we handle a redirect, which made the op_cancel() call fail.  With the
coming Objecter changes, this will no longer be the case.  However, there
are also locking and threading changes that will make cancellation racy,
so we will not be able to rely on it always preventing the callback.
Either way, this will avoid the problem.

Fixes: #7588
Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index cf795ecc1d614fb1528af74cec58a1e01603d723..e031ceab70f5b5dca7ef0e252b2dbcaa86739d4c 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -5335,9 +5335,11 @@ struct C_Copyfrom : public Context {
   hobject_t oid;
   epoch_t last_peering_reset;
   ceph_tid_t tid;
-  C_Copyfrom(ReplicatedPG *p, hobject_t o, epoch_t lpr)
+  ReplicatedPG::CopyOpRef cop;
+  C_Copyfrom(ReplicatedPG *p, hobject_t o, epoch_t lpr,
+            const ReplicatedPG::CopyOpRef& c)
     : pg(p), oid(o), last_peering_reset(lpr),
-      tid(0)
+      tid(0), cop(c)
   {}
   void finish(int r) {
     if (r == -ECANCELED)
@@ -5584,7 +5586,7 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop)
              &cop->rval);
 
   C_Copyfrom *fin = new C_Copyfrom(this, obc->obs.oi.soid,
-                                  get_last_peering_reset());
+                                  get_last_peering_reset(), cop);
   gather.set_finisher(new C_OnFinisher(fin,
                                       &osd->objecter_finisher));
 
@@ -6015,8 +6017,10 @@ void ReplicatedPG::cancel_copy(CopyOpRef cop, bool requeue)
   if (cop->objecter_tid) {
     Mutex::Locker l(osd->objecter_lock);
     osd->objecter->op_cancel(cop->objecter_tid, -ECANCELED);
+    cop->objecter_tid = 0;
     if (cop->objecter_tid2) {
       osd->objecter->op_cancel(cop->objecter_tid2, -ECANCELED);
+      cop->objecter_tid2 = 0;
     }
   }
 
@@ -6406,6 +6410,7 @@ void ReplicatedPG::cancel_flush(FlushOpRef fop, bool requeue)
   if (fop->objecter_tid) {
     Mutex::Locker l(osd->objecter_lock);
     osd->objecter->op_cancel(fop->objecter_tid, -ECANCELED);
+    fop->objecter_tid = 0;
   }
   if (requeue) {
     if (fop->op)