objecter: refactor pool dne checks

We need to verify that a pool really doesn't exist before erroring out. In
particular, we need to make sure it doesn't exist in an osdmap we haven't
seen yet (future from this client's perspective, but still strictly in the
past).  The code to do this before was fragile and broken.  A 'map latest'
check would go out for every map epoch we process, and we would clear out
our check state on the first reply.  Also, we would have to win the race
to get a check back for the exact map that we have before we would succeed.

Instead:
 - If we haven't already done so, ask the monitor for the latest map.  This
   establishes an upper bound on the current map at the time the request
   was queued.
 - On reply, make note of this map.
 - Thereafter, we can fail the request if at any point we call into the
   check_*_pool_dne() helper and the current map is >= that bound map. The
   caller performs the actual check for whether the pool exists or not.

Among other things, this fixes testrados_list_parallel.

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

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index a7f7b2ac8ee98143235b214572d13f3c5a12e9d0..ee476d41caed0e3327a8005740bc0b7d1e181219 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -263,7 +263,7 @@ void Objecter::send_linger(LingerOp *info)
   if (info->session) {
     int r = recalc_op_target(o);
     if (r == RECALC_OP_TARGET_POOL_DNE) {
-      linger_check_for_latest_map(info);
+      check_linger_pool_dne(info);
     }
   }
 
@@ -407,7 +407,7 @@ void Objecter::scan_requests(bool skipped_map,
       linger_cancel_map_check(op);
       break;
     case RECALC_OP_TARGET_POOL_DNE:
-      linger_check_for_latest_map(op);
+      check_linger_pool_dne(op);
       break;
     }
   }
@@ -430,7 +430,7 @@ void Objecter::scan_requests(bool skipped_map,
       op_cancel_map_check(op);
       break;
     case RECALC_OP_TARGET_POOL_DNE:
-      op_check_for_latest_map(op);
+      check_op_pool_dne(op);
       break;
     }
   }
@@ -598,6 +598,8 @@ void Objecter::handle_osd_map(MOSDMap *m)
 
 void Objecter::C_Op_Map_Latest::finish(int r)
 {
+  lgeneric_subdout(objecter->cct, objecter, 10) << "op_map_latest r=" << r << " tid=" << tid
+                                               << " latest " << latest << dendl;
   if (r == -EAGAIN) {
     // ignore callback; we will retry in resend_mon_ops()
     return;
@@ -608,22 +610,50 @@ void Objecter::C_Op_Map_Latest::finish(int r)
   map<tid_t, Op*>::iterator iter =
     objecter->check_latest_map_ops.find(tid);
   if (iter == objecter->check_latest_map_ops.end()) {
+    lgeneric_subdout(objecter->cct, objecter, 10) << "op_map_latest op " << tid << " not found" << dendl;
     return;
   }
 
   Op *op = iter->second;
   objecter->check_latest_map_ops.erase(iter);
 
-  if (r == 0) { // we had the latest map
-    if (op->onack) {
-      op->onack->complete(-ENOENT);
+  lgeneric_subdout(objecter->cct, objecter, 20) << "op_map_latest op " << op << dendl;
+
+  if (op->map_dne_bound == 0)
+    op->map_dne_bound = latest;
+
+  objecter->check_op_pool_dne(op);
+}
+
+void Objecter::check_op_pool_dne(Op *op)
+{
+  ldout(cct, 10) << "check_op_pool_dne tid " << op->tid
+                << " current " << osdmap->get_epoch()
+                << " map_dne_bound " << op->map_dne_bound
+                << dendl;
+  if (op->map_dne_bound > 0) {
+    if (osdmap->get_epoch() >= op->map_dne_bound) {
+      // we had a new enough map
+      ldout(cct, 10) << "check_op_pool_dne tid " << op->tid
+                    << " concluding pool " << op->pgid.pool() << " dne"
+                    << dendl;
+      if (op->onack) {
+       op->onack->complete(-ENOENT);
+      }
+      if (op->oncommit) {
+       op->oncommit->complete(-ENOENT);
+      }
+      op->session_item.remove_myself();
+      ops.erase(op->tid);
+      delete op;
     }
-    if (op->oncommit) {
-      op->oncommit->complete(-ENOENT);
+  } else {
+    // ask the monitor
+    if (check_latest_map_ops.count(op->tid) == 0) {
+      check_latest_map_ops[op->tid] = op;
+      C_Op_Map_Latest *c = new C_Op_Map_Latest(this, op->tid);
+      monc->get_version("osdmap", &c->latest, NULL, c);
     }
-    op->session_item.remove_myself();
-    objecter->ops.erase(op->tid);
-    delete op;
   }
 }
 
@@ -644,32 +674,39 @@ void Objecter::C_Linger_Map_Latest::finish(int r)
 
   LingerOp *op = iter->second;
   objecter->check_latest_map_lingers.erase(iter);
-
-  if (r == 0) { // we had the latest map
-    if (op->on_reg_ack) {
-      op->on_reg_ack->complete(-ENOENT);
-    }
-    if (op->on_reg_commit) {
-      op->on_reg_commit->complete(-ENOENT);
-    }
-    objecter->unregister_linger(op->linger_id);
-  }
   op->put();
-}
 
-void Objecter::op_check_for_latest_map(Op *op)
-{
-  check_latest_map_ops[op->tid] = op;
-  monc->is_latest_map("osdmap", osdmap->get_epoch(),
-                     new C_Op_Map_Latest(this, op->tid));
+  if (op->map_dne_bound == 0)
+    op->map_dne_bound = latest;
+
+  objecter->check_linger_pool_dne(op);
 }
 
-void Objecter::linger_check_for_latest_map(LingerOp *op)
+void Objecter::check_linger_pool_dne(LingerOp *op)
 {
-  op->get();
-  check_latest_map_lingers[op->linger_id] = op;
-  monc->is_latest_map("osdmap", osdmap->get_epoch(),
-                     new C_Linger_Map_Latest(this, op->linger_id));
+  ldout(cct, 10) << "check_linger_pool_dne linger_id " << op->linger_id
+                << " current " << osdmap->get_epoch()
+                << " map_dne_bound " << op->map_dne_bound
+                << dendl;
+  if (op->map_dne_bound > 0) {
+    if (osdmap->get_epoch() >= op->map_dne_bound) {
+      if (op->on_reg_ack) {
+       op->on_reg_ack->complete(-ENOENT);
+      }
+      if (op->on_reg_commit) {
+       op->on_reg_commit->complete(-ENOENT);
+      }
+      unregister_linger(op->linger_id);
+    }
+  } else {
+    // ask the monitor
+    if (check_latest_map_lingers.count(op->linger_id) == 0) {
+      op->get();
+      check_latest_map_lingers[op->linger_id] = op;
+      C_Linger_Map_Latest *c = new C_Linger_Map_Latest(this, op->linger_id);
+      monc->get_version("osdmap", &c->latest, NULL, c);
+    }
+  }
 }
 
 void Objecter::op_cancel_map_check(Op *op)
@@ -890,15 +927,15 @@ void Objecter::resend_mon_ops()
   for (map<tid_t, Op*>::iterator p = check_latest_map_ops.begin();
        p != check_latest_map_ops.end();
        ++p) {
-    monc->is_latest_map("osdmap", osdmap->get_epoch(),
-                       new C_Op_Map_Latest(this, p->second->tid));
+    C_Op_Map_Latest *c = new C_Op_Map_Latest(this, p->second->tid);
+    monc->get_version("osdmap", &c->latest, NULL, c);
   }
 
   for (map<uint64_t, LingerOp*>::iterator p = check_latest_map_lingers.begin();
        p != check_latest_map_lingers.end();
        ++p) {
-    monc->is_latest_map("osdmap", osdmap->get_epoch(),
-                       new C_Linger_Map_Latest(this, p->second->linger_id));
+    C_Linger_Map_Latest *c = new C_Linger_Map_Latest(this, p->second->linger_id);
+    monc->get_version("osdmap", &c->latest, NULL, c);
   }
 }
 
@@ -1024,7 +1061,7 @@ tid_t Objecter::_op_submit(Op *op)
   }
 
   if (check_for_latest_map) {
-    op_check_for_latest_map(op);
+    check_op_pool_dne(op);
   }
 
   ldout(cct, 5) << num_unacked << " unacked, " << num_uncommitted << " uncommitted" << dendl;
@@ -1050,6 +1087,7 @@ int Objecter::recalc_op_target(Op *op)
   vector<int> acting;
   pg_t pgid = op->pgid;
   if (op->precalc_pgid) {
+    ldout(cct, 10) << "recalc_op_target have " << pgid << " pool " << osdmap->have_pg_pool(pgid.pool()) << dendl;
     if (!osdmap->have_pg_pool(pgid.pool()))
       return RECALC_OP_TARGET_POOL_DNE;
   } else {

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index b23c0c50596768c0711d85e2974c3dc41e26d3d9..243868f4035d125da957da7bb0abdc774f3de698 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -615,6 +615,7 @@ public:
     utime_t stamp;
 
     bool precalc_pgid;
+    epoch_t map_dne_bound;
 
     bool budgeted;
 
@@ -631,6 +632,7 @@ public:
       flags(f), priority(0), onack(ac), oncommit(co),
       tid(0), attempts(0),
       paused(false), objver(ov), reply_epoch(NULL), precalc_pgid(false),
+      map_dne_bound(0),
       budgeted(false),
       should_resend(true) {
       ops.swap(op);
@@ -657,7 +659,8 @@ public:
   struct C_Op_Map_Latest : public Context {
     Objecter *objecter;
     tid_t tid;
-    C_Op_Map_Latest(Objecter *o, tid_t t) : objecter(o), tid(t) {}
+    version_t latest;
+    C_Op_Map_Latest(Objecter *o, tid_t t) : objecter(o), tid(t), latest(0) {}
     void finish(int r);
   };
 
@@ -800,12 +803,14 @@ public:
     xlist<LingerOp*>::item session_item;
 
     tid_t register_tid;
+    epoch_t map_dne_bound;
 
     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) {}
+                register_tid(0),
+                map_dne_bound(0) {}
 
     // no copy!
     const LingerOp &operator=(const LingerOp& r);
@@ -845,8 +850,9 @@ public:
   struct C_Linger_Map_Latest : public Context {
     Objecter *objecter;
     uint64_t linger_id;
+    version_t latest;
     C_Linger_Map_Latest(Objecter *o, uint64_t id) :
-      objecter(o), linger_id(id) {}
+      objecter(o), linger_id(id), latest(0) {}
     void finish(int r);
   };
 
@@ -895,9 +901,9 @@ public:
   void _linger_ack(LingerOp *info, int r);
   void _linger_commit(LingerOp *info, int r);
 
-  void op_check_for_latest_map(Op *op);
+  void check_op_pool_dne(Op *op);
   void op_cancel_map_check(Op *op);
-  void linger_check_for_latest_map(LingerOp *op);
+  void check_linger_pool_dne(LingerOp *op);
   void linger_cancel_map_check(LingerOp *op);
 
   void kick_requests(OSDSession *session);