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>

2 files changed, 21 insertions, 3 deletions

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 2491ea7ab41..9d7fe67cf9d 100644
--- 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 9a20849d574..e23f32a8d0f 100644
--- 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);