From cce58a31e8cd0dc53591c57d12f5594bb891db56 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 16 Oct 2012 16:30:25 -0700 Subject: [PATCH] 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 --- src/osdc/Objecter.cc | 112 +++++++++++++++++++++++++++++-------------- src/osdc/Objecter.h | 16 +++++-- 2 files changed, 86 insertions(+), 42 deletions(-) diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index a7f7b2ac8ee98..ee476d41caed0 100644 --- 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::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::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::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 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 b23c0c5059676..243868f4035d1 100644 --- 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::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); -- 2.39.5