osdc/Objecter: distinguish between multiple notify completions

We may send a notify to the cluster multiple times due to OSDMap
changes.  In some cases, earlier notify attempts may complete with
an error, while later attempts succeed.  We need to only pay
attention to the most-recently send notify's completion.

Do this by making note of the notify_id in the initial ACK (only
present when talking to newer OSDs).  When we get a notify
completion, match it against our expected notify_id (if we have
one) or else discard it.

This is important because in some cases an early notify completion
may be an error while a later one succeeds.

Note that if we are talking to an old cluster we will simply not record a
notify_id and our behavior will be the same as before (we will trust any
notify completion we get).

Fixes: #13114
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit e86d033854c76f344c678e92016c4e5c5e0385e2)

Conflicts:
	src/osdc/Objecter.cc
		In Objecter::handle_watch_notify, a conflict was there due to a modified comment by commit 47277c51db7bb2725ea117e4e8834869ae93e006, which was not backported

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 856425af9553a679c5c1fd266cf724aaee5d054d..23fef147d36fb87a9c90a35bfa0db1610a4fab6b 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -419,6 +419,7 @@ void Objecter::_send_linger(LingerOp *info)
   vector<OSDOp> opv;
   Context *oncommit = NULL;
   info->watch_lock.get_read(); // just to read registered status
+  bufferlist *poutbl = NULL;
   if (info->registered && info->is_watch) {
     ldout(cct, 15) << "send_linger " << info->linger_id << " reconnect" << dendl;
     opv.push_back(OSDOp());
@@ -430,7 +431,12 @@ void Objecter::_send_linger(LingerOp *info)
   } else {
     ldout(cct, 15) << "send_linger " << info->linger_id << " register" << dendl;
     opv = info->ops;
-    oncommit = new C_Linger_Commit(this, info);
+    C_Linger_Commit *c = new C_Linger_Commit(this, info);
+    if (!info->is_watch) {
+      info->notify_id = 0;
+      poutbl = &c->outbl;
+    }
+    oncommit = c;
   }
   info->watch_lock.put_read();
   Op *o = new Op(info->target.base_oid, info->target.base_oloc,
@@ -438,6 +444,7 @@ void Objecter::_send_linger(LingerOp *info)
                 NULL, NULL,
                 info->pobjver);
   o->oncommit_sync = oncommit;
+  o->outbl = poutbl;
   o->snapid = info->snap;
   o->snapc = info->snapc;
   o->mtime = info->mtime;
@@ -467,7 +474,7 @@ void Objecter::_send_linger(LingerOp *info)
   logger->inc(l_osdc_linger_send);
 }
 
-void Objecter::_linger_commit(LingerOp *info, int r) 
+void Objecter::_linger_commit(LingerOp *info, int r, bufferlist& outbl)
 {
   RWLock::WLocker wl(info->watch_lock);
   ldout(cct, 10) << "_linger_commit " << info->linger_id << dendl;
@@ -479,6 +486,17 @@ void Objecter::_linger_commit(LingerOp *info, int r)
   // only tell the user the first time we do this
   info->registered = true;
   info->pobjver = NULL;
+
+  if (!info->is_watch) {
+    // make note of the notify_id
+    bufferlist::iterator p = outbl.begin();
+    try {
+      ::decode(info->notify_id, p);
+      ldout(cct, 10) << "_linger_commit  notify_id=" << info->notify_id << dendl;
+    }
+    catch (buffer::error& e) {
+    }
+  }
 }
 
 struct C_DoWatchError : public Context {
@@ -777,11 +795,17 @@ void Objecter::handle_watch_notify(MWatchNotify *m)
       }
     }
   } else if (!info->is_watch) {
-    // notify completion; we can do this inline since we know the only user
-    // (librados) is safe to call in fast-dispatch context
-    assert(info->on_notify_finish);
-    info->notify_result_bl->claim(m->get_data());
-    info->on_notify_finish->complete(m->return_code);
+    // we have CEPH_WATCH_EVENT_NOTIFY_COMPLETE; we can do this inline since
+    // we know the only user (librados) is safe to call in fast-dispatch context
+    if (info->notify_id &&
+       info->notify_id != m->notify_id) {
+      ldout(cct, 10) << __func__ << " reply notify " << m->notify_id
+                    << " != " << info->notify_id << ", ignoring" << dendl;
+    } else {
+      assert(info->on_notify_finish);
+      info->notify_result_bl->claim(m->get_data());
+      info->on_notify_finish->complete(m->return_code);
+    }
   } else {
     finisher->queue(new C_DoWatchNotify(this, info, m));
     _linger_callback_queue();

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index b9fd0cd6c12bd097cf483940417c75f213cd0c93..6dbc491b4abfd79c6d8d6206934ab0fb56b9c633 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -1561,6 +1561,7 @@ public:
     // we trigger these from an async finisher
     Context *on_notify_finish;
     bufferlist *notify_result_bl;
+    uint64_t notify_id;
 
     WatchContext *watch_context;
 
@@ -1595,6 +1596,7 @@ public:
                 on_reg_commit(NULL),
                 on_notify_finish(NULL),
                 notify_result_bl(NULL),
+                notify_id(0),
                 watch_context(NULL),
                 session(NULL),
                 register_tid(0),
@@ -1619,6 +1621,7 @@ public:
   struct C_Linger_Commit : public Context {
     Objecter *objecter;
     LingerOp *info;
+    bufferlist outbl;  // used for notify only
     C_Linger_Commit(Objecter *o, LingerOp *l) : objecter(o), info(l) {
       info->get();
     }
@@ -1626,7 +1629,7 @@ public:
       info->put();
     }
     void finish(int r) {
-      objecter->_linger_commit(info, r);
+      objecter->_linger_commit(info, r, outbl);
     }
   };
 
@@ -1771,7 +1774,7 @@ public:
 
   void _linger_submit(LingerOp *info);
   void _send_linger(LingerOp *info);
-  void _linger_commit(LingerOp *info, int r);
+  void _linger_commit(LingerOp *info, int r, bufferlist& outbl);
   void _linger_reconnect(LingerOp *info, int r);
   void _send_linger_ping(LingerOp *info);
   void _linger_ping(LingerOp *info, int r, utime_t sent, uint32_t register_gen);