From: Samuel Just Date: Thu, 2 Feb 2017 21:14:09 +0000 (-0800) Subject: osd_types: refactor PriorSet so it can be tested without an OSDMap X-Git-Tag: v12.0.3~129^2~30 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5a9c3be3c7f646d01d27d408be80b6e9ca7e0b90;p=ceph.git osd_types: refactor PriorSet so it can be tested without an OSDMap Signed-off-by: Samuel Just --- diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 37b246dc0d8..1d61ac8bfbc 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -841,11 +841,29 @@ PastIntervals::PriorSet PG::build_prior() } } + const OSDMap &osdmap = *get_osdmap(); PastIntervals::PriorSet prior = past_intervals.get_prior_set( pool.info.ec_pool(), info.history.last_epoch_started, get_pgbackend()->get_is_recoverable_predicate(), - *get_osdmap(), + [&](epoch_t start, int osd, epoch_t *lost_at) { + const osd_info_t *pinfo = 0; + if (osdmap.exists(osd)) { + pinfo = &osdmap.get_info(osd); + if (lost_at) + *lost_at = pinfo->lost_at; + } + + if (osdmap.is_up(osd)) { + return PastIntervals::UP; + } else if (!pinfo) { + return PastIntervals::DNE; + } else if (pinfo->lost_at > start) { + return PastIntervals::LOST; + } else { + return PastIntervals::DOWN; + } + }, up, acting, this); diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc index 7a10675bafe..c3d60c20104 100644 --- a/src/osd/osd_types.cc +++ b/src/osd/osd_types.cc @@ -2891,10 +2891,9 @@ public: return make_pair(make_pair(0, 0), 0); } } - set get_might_have_unfound( - pg_shard_t pg_whoami, + set get_all_participants( bool ec_pool) const override { - set might_have_unfound; + set all_participants; // We need to decide who might have unfound objects that we need auto p = interval_map.rbegin(); @@ -2910,11 +2909,11 @@ public: std::vector::const_iterator a_end = interval.acting.end(); for (; a != a_end; ++a, ++i) { pg_shard_t shard(*a, ec_pool ? shard_id_t(i) : shard_id_t::NO_SHARD); - if (*a != CRUSH_ITEM_NONE && shard != pg_whoami) - might_have_unfound.insert(shard); + if (*a != CRUSH_ITEM_NONE) + all_participants.insert(shard); } } - return might_have_unfound; + return all_participants; } void add_interval( bool ec_pool, @@ -2955,26 +2954,25 @@ public: new pi_simple_rep( true, ivallst { ival{{0, 1, 2}, {0, 1, 2}, 10, 20, true, 0, 0} - , ival{{ 1, 2}, { 1, 2}, 20, 30, true, 1, 1} - , ival{{ 2}, { 2}, 30, 35, false, 2, 2} - , ival{{0, 2}, {0, 2}, 35, 50, true, 0, 0} + , ival{{ 1, 2}, { 1, 2}, 21, 30, true, 1, 1} + , ival{{ 2}, { 2}, 31, 35, false, 2, 2} + , ival{{0, 2}, {0, 2}, 36, 50, true, 0, 0} })); o.push_back( new pi_simple_rep( false, ivallst { ival{{0, 1, 2}, {0, 1, 2}, 10, 20, true, 0, 0} , ival{{ 1, 2}, { 1, 2}, 20, 30, true, 1, 1} - , ival{{ 2}, { 2}, 30, 35, false, 2, 2} - , ival{{0, 2}, {0, 2}, 35, 50, true, 0, 0} + , ival{{ 2}, { 2}, 31, 35, false, 2, 2} + , ival{{0, 2}, {0, 2}, 36, 50, true, 0, 0} })); o.push_back( new pi_simple_rep( true, ivallst { ival{{2, 1, 0}, {2, 1, 0}, 10, 20, true, 1, 1} - , ival{{ 0, 2}, { 0, 2}, 20, 30, true, 0, 0} - , ival{{ 0, 2}, { 0}, 30, 35, false, 2, 2} - , ival{{ 0, 2}, {2, 0}, 30, 35, true, 2, 2} - , ival{{ 0, 2}, { 0, 2}, 35, 50, true, 0, 0} + , ival{{ 0, 2}, { 0, 2}, 21, 30, true, 0, 0} + , ival{{ 0, 2}, {2, 0}, 31, 35, true, 2, 2} + , ival{{ 0, 2}, { 0, 2}, 36, 50, true, 0, 0} })); return; } @@ -2983,6 +2981,8 @@ public: epoch_t les, std::function &)> &&f) const override { for (auto i = interval_map.rbegin(); i != interval_map.rend(); ++i) { + if (!i->second.maybe_went_rw) + continue; if (i->second.last < les) break; set actingset; @@ -3073,7 +3073,7 @@ WRITE_CLASS_ENCODER(compact_interval_t) class pi_compact_rep : public PastIntervals::interval_rep { epoch_t start = 0; epoch_t end = 0; // inclusive - set might_have_unfound; + set all_participants; list intervals; pi_compact_rep( @@ -3097,10 +3097,9 @@ public: pair, epoch_t> get_bounds() const override { return make_pair(make_pair(start, start), end); } - set get_might_have_unfound( - pg_shard_t pg_whoami, + set get_all_participants( bool ec_pool) const override { - return might_have_unfound; + return all_participants; } void add_interval( bool ec_pool, const PastIntervals::pg_interval_t &interval) override { @@ -3117,7 +3116,7 @@ public: interval.acting[i], ec_pool ? shard_id_t(i) : shard_id_t::NO_SHARD)); } - might_have_unfound.insert(acting.begin(), acting.end()); + all_participants.insert(acting.begin(), acting.end()); if (!interval.maybe_went_rw) return; intervals.push_back( @@ -3172,26 +3171,25 @@ public: new pi_compact_rep( true, ivallst { ival{{0, 1, 2}, {0, 1, 2}, 10, 20, true, 0, 0} - , ival{{ 1, 2}, { 1, 2}, 20, 30, true, 1, 1} - , ival{{ 2}, { 2}, 30, 35, false, 2, 2} - , ival{{0, 2}, {0, 2}, 35, 50, true, 0, 0} + , ival{{ 1, 2}, { 1, 2}, 21, 30, true, 1, 1} + , ival{{ 2}, { 2}, 31, 35, false, 2, 2} + , ival{{0, 2}, {0, 2}, 36, 50, true, 0, 0} })); o.push_back( new pi_compact_rep( false, ivallst { ival{{0, 1, 2}, {0, 1, 2}, 10, 20, true, 0, 0} - , ival{{ 1, 2}, { 1, 2}, 20, 30, true, 1, 1} - , ival{{ 2}, { 2}, 30, 35, false, 2, 2} - , ival{{0, 2}, {0, 2}, 35, 50, true, 0, 0} + , ival{{ 1, 2}, { 1, 2}, 21, 30, true, 1, 1} + , ival{{ 2}, { 2}, 31, 35, false, 2, 2} + , ival{{0, 2}, {0, 2}, 36, 50, true, 0, 0} })); o.push_back( new pi_compact_rep( true, ivallst { ival{{2, 1, 0}, {2, 1, 0}, 10, 20, true, 1, 1} - , ival{{ 0, 2}, { 0, 2}, 20, 30, true, 0, 0} - , ival{{ 0, 2}, { 0}, 30, 35, false, 2, 2} - , ival{{ 0, 2}, {2, 0}, 30, 35, true, 2, 2} - , ival{{ 0, 2}, { 0, 2}, 35, 50, true, 0, 0} + , ival{{ 0, 2}, { 0, 2}, 21, 30, true, 0, 0} + , ival{{ 0, 2}, {2, 0}, 31, 35, true, 2, 2} + , ival{{ 0, 2}, { 0, 2}, 36, 50, true, 0, 0} })); } void iterate_mayberw_back_to( @@ -3226,6 +3224,17 @@ ostream& operator<<(ostream& out, const PastIntervals &i) return i.past_intervals->print(out); } +ostream& operator<<(ostream& out, const PastIntervals::PriorSet &i) +{ + return out << "PriorSet(" + << "ec_pool: " << i.ec_pool + << ", probe: " << i.probe + << ", down: " << i.down + << ", blocked_by: " << i.blocked_by + << ", pg_down: " << i.pg_down + << ")"; +} + void PastIntervals::decode(bufferlist::iterator &bl) { DECODE_START(1, bl); @@ -3507,142 +3516,6 @@ bool PastIntervals::check_new_interval( } } -PastIntervals::PriorSet::PriorSet( - const PastIntervals &past_intervals, - bool ec_pool, - epoch_t last_epoch_started, - IsPGRecoverablePredicate *c, - const OSDMap &osdmap, - const vector &up, - const vector &acting, - const DoutPrefixProvider *dpp) - : ec_pool(ec_pool), pg_down(false), pcontdec(c) -{ - /* - * We have to be careful to gracefully deal with situations like - * so. Say we have a power outage or something that takes out both - * OSDs, but the monitor doesn't mark them down in the same epoch. - * The history may look like - * - * 1: A B - * 2: B - * 3: let's say B dies for good, too (say, from the power spike) - * 4: A - * - * which makes it look like B may have applied updates to the PG - * that we need in order to proceed. This sucks... - * - * To minimize the risk of this happening, we CANNOT go active if - * _any_ OSDs in the prior set are down until we send an MOSDAlive - * to the monitor such that the OSDMap sets osd_up_thru to an epoch. - * Then, we have something like - * - * 1: A B - * 2: B up_thru[B]=0 - * 3: - * 4: A - * - * -> we can ignore B, bc it couldn't have gone active (alive_thru - * still 0). - * - * or, - * - * 1: A B - * 2: B up_thru[B]=0 - * 3: B up_thru[B]=2 - * 4: - * 5: A - * - * -> we must wait for B, bc it was alive through 2, and could have - * written to the pg. - * - * If B is really dead, then an administrator will need to manually - * intervene by marking the OSD as "lost." - */ - - // Include current acting and up nodes... not because they may - // contain old data (this interval hasn't gone active, obviously), - // but because we want their pg_info to inform choose_acting(), and - // so that we know what they do/do not have explicitly before - // sending them any new info/logs/whatever. - for (unsigned i = 0; i < acting.size(); i++) { - if (acting[i] != CRUSH_ITEM_NONE) - probe.insert(pg_shard_t(acting[i], ec_pool ? shard_id_t(i) : shard_id_t::NO_SHARD)); - } - // It may be possible to exclude the up nodes, but let's keep them in - // there for now. - for (unsigned i = 0; i < up.size(); i++) { - if (up[i] != CRUSH_ITEM_NONE) - probe.insert(pg_shard_t(up[i], ec_pool ? shard_id_t(i) : shard_id_t::NO_SHARD)); - } - - past_intervals.iterate_mayberw_back_to( - ec_pool, - last_epoch_started, - [&](epoch_t start, const set &acting) { - ldpp_dout(dpp, 10) << "build_prior maybe_rw interval:" << start - << ", acting: " << acting << dendl; - - // look at candidate osds during this interval. each falls into - // one of three categories: up, down (but potentially - // interesting), or lost (down, but we won't wait for it). - set up_now; - // any candidates down now (that might have useful data) - bool any_down_now = false; - - // consider ACTING osds - for (auto &&so: acting) { - const osd_info_t *pinfo = 0; - if (osdmap.exists(so.osd)) - pinfo = &osdmap.get_info(so.osd); - - if (osdmap.is_up(so.osd)) { - // include past acting osds if they are up. - probe.insert(so); - up_now.insert(so); - } else if (!pinfo) { - ldpp_dout(dpp, 10) << "build_prior prior osd." << so.osd - << " no longer exists" << dendl; - down.insert(so.osd); - } else if (pinfo->lost_at > start) { - ldpp_dout(dpp, 10) << "build_prior prior osd." << so.osd - << " is down, but lost_at " << pinfo->lost_at << dendl; - up_now.insert(so); - down.insert(so.osd); - } else { - ldpp_dout(dpp, 10) << "build_prior prior osd." << so.osd - << " is down" << dendl; - down.insert(so.osd); - any_down_now = true; - } - } - - // if not enough osds survived this interval, and we may have gone rw, - // then we need to wait for one of those osds to recover to - // ensure that we haven't lost any information. - if (!(*pcontdec)(up_now) && any_down_now) { - // fixme: how do we identify a "clean" shutdown anyway? - ldpp_dout(dpp, 10) << "build_prior possibly went active+rw, insufficient up;" - << " including down osds" << dendl; - for (auto &&so: acting) { - if (osdmap.exists(so.osd) && // if it doesn't exist, we already consider it lost. - osdmap.is_down(so.osd)) { - pg_down = true; - - // make note of when any down osd in the cur set was lost, so that - // we can notice changes in prior_set_affected. - blocked_by[so.osd] = osdmap.get_info(so.osd).lost_at; - } - } - } - }); - - ldpp_dout(dpp, 10) << "build_prior final: probe " << probe - << " down " << down - << " blocked_by " << blocked_by - << (pg_down ? " pg_down":"") - << dendl; -} // true if the given map affects the prior set bool PastIntervals::PriorSet::affected_by_map( @@ -3703,16 +3576,6 @@ bool PastIntervals::PriorSet::affected_by_map( return false; } -std::ostream& operator<<( - std::ostream& oss, - const PastIntervals::PriorSet &prior) -{ - oss << "PriorSet[probe=" << prior.probe << " " - << "down=" << prior.down << " " - << "blocked_by=" << prior.blocked_by << "]"; - return oss; -} - ostream& operator<<(ostream& out, const PastIntervals::pg_interval_t& i) { out << "interval(" << i.first << "-" << i.last diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index ea4410c6b18..849e66b4f26 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -2529,8 +2529,7 @@ public: virtual bool empty() const = 0; virtual void clear() = 0; virtual pair, epoch_t> get_bounds() const = 0; - virtual set get_might_have_unfound( - pg_shard_t pg_whoami, + virtual set get_all_participants( bool ec_pool) const = 0; virtual void add_interval(bool ec_pool, const pg_interval_t &interval) = 0; virtual unique_ptr clone() const = 0; @@ -2562,6 +2561,11 @@ private: PastIntervals(interval_rep *rep) : past_intervals(rep) {} public: + void add_interval(bool ec_pool, const pg_interval_t &interval) { + assert(past_intervals); + return past_intervals->add_interval(ec_pool, interval); + } + bool is_classic() const { assert(past_intervals); return past_intervals->is_classic(); @@ -2686,14 +2690,25 @@ public: } /** - * Return all osds which have been in the acting set back to the - * latest epoch to which we have trimmed + * Return all shards which have been in the acting set back to the + * latest epoch to which we have trimmed except for pg_whoami */ set get_might_have_unfound( pg_shard_t pg_whoami, bool ec_pool) const { assert(past_intervals); - return past_intervals->get_might_have_unfound(pg_whoami, ec_pool); + auto ret = past_intervals->get_all_participants(ec_pool); + ret.erase(pg_whoami); + return ret; + } + + /** + * Return all shards which we might want to talk to for peering + */ + set get_all_probe( + bool ec_pool) const { + assert(past_intervals); + return past_intervals->get_all_participants(ec_pool); } /* Return the set of epochs @@ -2707,6 +2722,12 @@ public: return past_intervals->get_bounds(); } + enum osd_state_t { + UP, + DOWN, + DNE, + LOST + }; struct PriorSet { bool ec_pool = false; set probe; /// current+prior OSDs we need to probe. @@ -2723,17 +2744,37 @@ public: PriorSet &operator=(const PriorSet &) = delete; PriorSet(const PriorSet &) = delete; + bool operator==(const PriorSet &rhs) const { + return (ec_pool == rhs.ec_pool) && + (probe == rhs.probe) && + (down == rhs.down) && + (blocked_by == rhs.blocked_by) && + (pg_down == rhs.pg_down); + } + bool affected_by_map( const OSDMap &osdmap, const DoutPrefixProvider *dpp) const; + // For verifying tests + PriorSet( + bool ec_pool, + set probe, + set down, + map blocked_by, + bool pg_down, + IsPGRecoverablePredicate *pcontdec) + : ec_pool(ec_pool), probe(probe), down(down), blocked_by(blocked_by), + pg_down(pg_down), pcontdec(pcontdec) {} + private: + template PriorSet( const PastIntervals &past_intervals, bool ec_pool, epoch_t last_epoch_started, IsPGRecoverablePredicate *c, - const OSDMap &osdmap, + F f, const vector &up, const vector &acting, const DoutPrefixProvider *dpp); @@ -2753,6 +2794,160 @@ WRITE_CLASS_ENCODER(PastIntervals) ostream& operator<<(ostream& out, const PastIntervals::pg_interval_t& i); ostream& operator<<(ostream& out, const PastIntervals &i); +ostream& operator<<(ostream& out, const PastIntervals::PriorSet &i); + +template +PastIntervals::PriorSet::PriorSet( + const PastIntervals &past_intervals, + bool ec_pool, + epoch_t last_epoch_started, + IsPGRecoverablePredicate *c, + F f, + const vector &up, + const vector &acting, + const DoutPrefixProvider *dpp) + : ec_pool(ec_pool), pg_down(false), pcontdec(c) +{ + /* + * We have to be careful to gracefully deal with situations like + * so. Say we have a power outage or something that takes out both + * OSDs, but the monitor doesn't mark them down in the same epoch. + * The history may look like + * + * 1: A B + * 2: B + * 3: let's say B dies for good, too (say, from the power spike) + * 4: A + * + * which makes it look like B may have applied updates to the PG + * that we need in order to proceed. This sucks... + * + * To minimize the risk of this happening, we CANNOT go active if + * _any_ OSDs in the prior set are down until we send an MOSDAlive + * to the monitor such that the OSDMap sets osd_up_thru to an epoch. + * Then, we have something like + * + * 1: A B + * 2: B up_thru[B]=0 + * 3: + * 4: A + * + * -> we can ignore B, bc it couldn't have gone active (alive_thru + * still 0). + * + * or, + * + * 1: A B + * 2: B up_thru[B]=0 + * 3: B up_thru[B]=2 + * 4: + * 5: A + * + * -> we must wait for B, bc it was alive through 2, and could have + * written to the pg. + * + * If B is really dead, then an administrator will need to manually + * intervene by marking the OSD as "lost." + */ + + // Include current acting and up nodes... not because they may + // contain old data (this interval hasn't gone active, obviously), + // but because we want their pg_info to inform choose_acting(), and + // so that we know what they do/do not have explicitly before + // sending them any new info/logs/whatever. + for (unsigned i = 0; i < acting.size(); i++) { + if (acting[i] != 0x7fffffff /* CRUSH_ITEM_NONE, can't import crush.h here */) + probe.insert(pg_shard_t(acting[i], ec_pool ? shard_id_t(i) : shard_id_t::NO_SHARD)); + } + // It may be possible to exclude the up nodes, but let's keep them in + // there for now. + for (unsigned i = 0; i < up.size(); i++) { + if (up[i] != 0x7fffffff /* CRUSH_ITEM_NONE, can't import crush.h here */) + probe.insert(pg_shard_t(up[i], ec_pool ? shard_id_t(i) : shard_id_t::NO_SHARD)); + } + + set all_probe = past_intervals.get_all_probe(ec_pool); + for (auto &&i: all_probe) { + switch (f(0, i.osd, nullptr)) { + case UP: { + probe.insert(i); + break; + } + case DNE: + case LOST: + case DOWN: { + down.insert(i.osd); + break; + } + } + } + + past_intervals.iterate_mayberw_back_to( + ec_pool, + last_epoch_started, + [&](epoch_t start, const set &acting) { + ldpp_dout(dpp, 10) << "build_prior maybe_rw interval:" << start + << ", acting: " << acting << dendl; + + // look at candidate osds during this interval. each falls into + // one of three categories: up, down (but potentially + // interesting), or lost (down, but we won't wait for it). + set up_now; + map candidate_blocked_by; + // any candidates down now (that might have useful data) + bool any_down_now = false; + + // consider ACTING osds + for (auto &&so: acting) { + epoch_t lost_at = 0; + switch (f(start, so.osd, &lost_at)) { + case UP: { + // include past acting osds if they are up. + up_now.insert(so); + break; + } + case DNE: { + ldpp_dout(dpp, 10) << "build_prior prior osd." << so.osd + << " no longer exists" << dendl; + break; + } + case LOST: { + ldpp_dout(dpp, 10) << "build_prior prior osd." << so.osd + << " is down, but lost_at " << lost_at << dendl; + up_now.insert(so); + break; + } + case DOWN: { + ldpp_dout(dpp, 10) << "build_prior prior osd." << so.osd + << " is down" << dendl; + candidate_blocked_by[so.osd] = lost_at; + any_down_now = true; + break; + } + } + } + + // if not enough osds survived this interval, and we may have gone rw, + // then we need to wait for one of those osds to recover to + // ensure that we haven't lost any information. + if (!(*pcontdec)(up_now) && any_down_now) { + // fixme: how do we identify a "clean" shutdown anyway? + ldpp_dout(dpp, 10) << "build_prior possibly went active+rw, insufficient up;" + << " including down osds" << dendl; + assert(!candidate_blocked_by.empty()); + pg_down = true; + blocked_by.insert( + candidate_blocked_by.begin(), + candidate_blocked_by.end()); + } + }); + + ldpp_dout(dpp, 10) << "build_prior final: probe " << probe + << " down " << down + << " blocked_by " << blocked_by + << (pg_down ? " pg_down":"") + << dendl; +} /** * pg_query_t - used to ask a peer for information about a pg.