From: Sage Weil Date: Sun, 20 Apr 2014 05:04:33 +0000 (-0700) Subject: osd: change in up set primary constitutes a peering interval change X-Git-Tag: v0.80-rc1~3^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=000233f7326d8bf36756bddc62f7f09d479208a6;p=ceph.git osd: change in up set primary constitutes a peering interval change In several places, a change in the up_primary triggers a new peering interval, but the palces that actually generate the new past intervals, including check_new_interval(), did not enforce that. This becomes somewhat obvious when you see that those callers are ignoring the up_primary output argument for pg_to_up_acting_osds(). Fix this by adding arguments to check_new_interval and fixing the callers to pass them in properly. Add a unit test case to verify this. Note that the past interval struct itself does not record who the up_primary was; possibly it should. Fixes: #8139 Signed-off-by: Sage Weil --- diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 4b9f3a36946c..2d6379055b23 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -2156,6 +2156,7 @@ struct pistate { vector old_acting, old_up; epoch_t same_interval_since; int primary; + int up_primary; }; void OSD::build_past_intervals_parallel() @@ -2207,9 +2208,10 @@ void OSD::build_past_intervals_parallel() continue; vector acting, up; + int up_primary; int primary; cur_map->pg_to_up_acting_osds( - pg->info.pgid.pgid, &up, 0, &acting, &primary); + pg->info.pgid.pgid, &up, &up_primary, &acting, &primary); if (p.same_interval_since == 0) { dout(10) << __func__ << " epoch " << cur_epoch << " pg " << pg->info.pgid @@ -2219,6 +2221,7 @@ void OSD::build_past_intervals_parallel() p.old_up = up; p.old_acting = acting; p.primary = primary; + p.up_primary = up_primary; continue; } assert(last_map); @@ -2228,6 +2231,8 @@ void OSD::build_past_intervals_parallel() p.primary, primary, p.old_acting, acting, + p.up_primary, + up_primary, p.old_up, up, p.same_interval_since, pg->info.history.last_epoch_clean, diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 32a7becd5b9d..db61ae5693e2 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -638,17 +638,19 @@ void PG::generate_past_intervals() OSDMapRef last_map, cur_map; int primary = -1; + int up_primary = -1; vector acting, up, old_acting, old_up; cur_map = osd->get_map(cur_epoch); cur_map->pg_to_up_acting_osds( - get_pgid().pgid, &up, 0, &acting, &primary); + get_pgid().pgid, &up, &up_primary, &acting, &primary); epoch_t same_interval_since = cur_epoch; dout(10) << __func__ << " over epochs " << cur_epoch << "-" << end_epoch << dendl; ++cur_epoch; for (; cur_epoch <= end_epoch; ++cur_epoch) { int old_primary = primary; + int old_up_primary = up_primary; last_map.swap(cur_map); old_up.swap(up); old_acting.swap(acting); @@ -657,7 +659,7 @@ void PG::generate_past_intervals() pg_t pgid = get_pgid().pgid; if (cur_map->get_pools().count(pgid.pool())) pgid = pgid.get_ancestor(cur_map->get_pg_num(pgid.pool())); - cur_map->pg_to_up_acting_osds(pgid, &up, 0, &acting, &primary); + cur_map->pg_to_up_acting_osds(pgid, &up, &up_primary, &acting, &primary); std::stringstream debug; bool new_interval = pg_interval_t::check_new_interval( @@ -665,6 +667,8 @@ void PG::generate_past_intervals() primary, old_acting, acting, + old_up_primary, + up_primary, old_up, up, same_interval_since, @@ -4686,6 +4690,8 @@ void PG::start_peering_interval( old_acting_primary.osd, new_acting_primary, oldacting, newacting, + old_up_primary.osd, + new_up_primary, oldup, newup, info.history.same_interval_since, info.history.last_epoch_clean, diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc index 33b1b9c34596..288527b74584 100644 --- a/src/osd/osd_types.cc +++ b/src/osd/osd_types.cc @@ -2157,10 +2157,12 @@ void pg_interval_t::generate_test_instances(list& o) } bool pg_interval_t::check_new_interval( - int old_primary, - int new_primary, + int old_acting_primary, + int new_acting_primary, const vector &old_acting, const vector &new_acting, + int old_up_primary, + int new_up_primary, const vector &old_up, const vector &new_up, epoch_t same_interval_since, @@ -2173,8 +2175,13 @@ bool pg_interval_t::check_new_interval( std::ostream *out) { // remember past interval - if (old_primary != new_primary || - new_acting != old_acting || new_up != old_up || + // NOTE: a change in the up set primary triggers an interval + // change, even though the interval members in the pg_interval_t + // do not change. + if (old_acting_primary != new_acting_primary || + new_acting != old_acting || + old_up_primary != new_up_primary || + new_up != old_up || (!(lastmap->get_pools().count(pool_id))) || (lastmap->get_pools().find(pool_id)->second.min_size != osdmap->get_pools().find(pool_id)->second.min_size) || @@ -2185,7 +2192,7 @@ bool pg_interval_t::check_new_interval( i.last = osdmap->get_epoch() - 1; i.acting = old_acting; i.up = old_up; - i.primary = old_primary; + i.primary = old_acting_primary; if (!i.acting.empty() && i.primary != -1 && i.acting.size() >= diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index 7874977ea92c..c8b7ff8675e8 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -1696,10 +1696,12 @@ struct pg_interval_t { * if an interval was closed out. */ static bool check_new_interval( - int old_primary, ///< [in] primary as of lastmap - int new_primary, ///< [in] primary as of lastmap + int old_acting_primary, ///< [in] primary as of lastmap + int new_acting_primary, ///< [in] primary as of lastmap const vector &old_acting, ///< [in] acting as of lastmap const vector &new_acting, ///< [in] acting as of osdmap + int old_up_primary, ///< [in] up primary of lastmap + int new_up_primary, ///< [in] up primary of osdmap const vector &old_up, ///< [in] up as of lastmap const vector &new_up, ///< [in] up as of osdmap epoch_t same_interval_since, ///< [in] as of osdmap diff --git a/src/test/osd/types.cc b/src/test/osd/types.cc index ac218fa80d3d..a04f2cba6132 100644 --- a/src/test/osd/types.cc +++ b/src/test/osd/types.cc @@ -155,6 +155,8 @@ TEST(pg_interval_t, check_new_interval) int new_primary = osd_id; vector new_up; new_up.push_back(osd_id); + int old_up_primary = osd_id; + int new_up_primary = osd_id; vector old_up = new_up; pg_t pgid; pgid.set_pool(pool_id); @@ -172,6 +174,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -200,6 +204,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -231,6 +237,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -263,6 +271,40 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, + old_up, + new_up, + same_interval_since, + last_epoch_clean, + osdmap, + lastmap, + pool_id, + pgid, + &past_intervals)); + ASSERT_EQ((unsigned int)1, past_intervals.size()); + ASSERT_EQ(same_interval_since, past_intervals[same_interval_since].first); + ASSERT_EQ(osdmap->get_epoch() - 1, past_intervals[same_interval_since].last); + ASSERT_EQ(osd_id, past_intervals[same_interval_since].acting[0]); + ASSERT_EQ(osd_id, past_intervals[same_interval_since].up[0]); + } + + // + // The up primary has changed + // + { + vector new_up; + int _new_up_primary = osd_id + 1; + + map past_intervals; + + ASSERT_TRUE(past_intervals.empty()); + ASSERT_TRUE(pg_interval_t::check_new_interval(old_primary, + new_primary, + old_acting, + new_acting, + old_up_primary, + _new_up_primary, old_up, new_up, same_interval_since, @@ -300,6 +342,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -337,6 +381,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -369,6 +415,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -419,6 +467,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -452,6 +502,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -495,6 +547,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since, @@ -542,6 +596,8 @@ TEST(pg_interval_t, check_new_interval) new_primary, old_acting, new_acting, + old_up_primary, + new_up_primary, old_up, new_up, same_interval_since,