objecter: don't use new tid when retrying notifies

Watches update the on-disk state in the OSD, and aren't idempotent,
so refreshing them must be treated as a separate transaction by the OSD.
Notifies are just in-memory state, and resending them will result in
acceptable behavior:

- if it's the same osd, the resent op will be recognized as a duplicate
- if it's a different osd, a new notify will be triggered since the new osd
  can't tell whether the original notify was received by any watchers

Using a new tid for each resend can cause some unecessary extra work,
as the first case turns into the second.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 2491ea7ab411f59c973a63c38f4473f24d6d3026..9d7fe67cf9d2e14505b2363a0170434c0d7001a1 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -267,7 +267,7 @@ void Objecter::send_linger(LingerOp *info)
   o->snapid = info->snap;
 
   // do not resend this; we will send a new op to reregister
-  o->should_resend = false;
+  o->should_resend = !info->is_watch;
 
   if (info->session) {
     int r = recalc_op_target(o);
@@ -278,7 +278,7 @@ void Objecter::send_linger(LingerOp *info)
 
   if (info->register_tid) {
     // repeat send.  cancel old registeration op, if any.
-    if (ops.count(info->register_tid)) {
+    if (info->is_watch && ops.count(info->register_tid)) {
       Op *o = ops[info->register_tid];
       cancel_op(o);
     }
@@ -335,6 +335,11 @@ void Objecter::unregister_linger(uint64_t linger_id)
   }
 }
 
+/**
+ * Note that this is meant to handle a watch OR a notify, but not both in the same ObjectOperation.
+ * This is because watches need to be resent with a new tid on map changes, while notifies
+ * need to resend using the old tid.
+ */
 tid_t Objecter::linger(const object_t& oid, const object_locator_t& oloc, 
                       ObjectOperation& op,
                       snapid_t snap, bufferlist& inbl, bufferlist *poutbl, int flags,
@@ -349,6 +354,18 @@ tid_t Objecter::linger(const object_t& oid, const object_locator_t& oloc,
   info->snap = snap;
   info->flags = flags;
   info->ops = op.ops;
+  bool saw_notify = false;
+  for (vector<OSDOp>::const_iterator it = info->ops.begin();
+       it != info->ops.end(); ++it) {
+    if (it->op.op == CEPH_OSD_OP_WATCH)
+      info->is_watch = true;
+    if (it->op.op == CEPH_OSD_OP_NOTIFY)
+      saw_notify = true;
+    if (info->is_watch)
+      assert(it->op.op != CEPH_OSD_OP_NOTIFY);
+    if (saw_notify)
+      assert(it->op.op != CEPH_OSD_OP_WATCH);
+  }
   info->inbl = inbl;
   info->poutbl = poutbl;
   info->pobjver = objver;

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 9a20849d57413fe856b8ceae53da3dad16bc73d1..e23f32a8d0f0822a383386d59dacf282bb3b42f5 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -816,13 +816,14 @@ public:
 
     tid_t register_tid;
     epoch_t map_dne_bound;
+    bool is_watch;
 
     LingerOp() : linger_id(0), flags(0), poutbl(NULL), pobjver(NULL),
                 registered(false),
                 on_reg_ack(NULL), on_reg_commit(NULL),
                 session(NULL), session_item(this),
                 register_tid(0),
-                map_dne_bound(0) {}
+                map_dne_bound(0), is_watch(false) {}
 
     // no copy!
     const LingerOp &operator=(const LingerOp& r);