From 18d2bb9bc272a24dbfb776e658f5a18cca7441f1 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Thu, 12 May 2016 09:58:57 -0400 Subject: [PATCH] osd/OSDMap: fix clean_temps and test For both pg_temp and primary_temp, we want to calculate what the mapping *will* be after the inc is applied, and if the mapping is redundant (it matches what we would have gotten anyway), remove it. Removing the mapping might mean putting a 'remove' entry in the inc, or it might mean removing the proposed addition in the inc. Update the unit test to test both cases. Signed-off-by: Sage Weil --- src/osd/OSDMap.cc | 52 ++++++++++++++++++-------------------- src/test/osd/TestOSDMap.cc | 41 +++++++++++++++++++----------- 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index a6fcd3810b0fe..cf1c21528cb86 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -1210,45 +1210,43 @@ void OSDMap::clean_temps(CephContext *cct, continue; } // redundant pg_temp? - if (pending_inc->new_pg_temp.count(p->first) == 0) { - vector raw_up; - int primary; - tmpmap.pg_to_raw_up(p->first, &raw_up, &primary); - if (raw_up == p->second) { - ldout(cct, 10) << __func__ << " removing pg_temp " << p->first << " " - << p->second << " that matches raw_up mapping" << dendl; - pending_inc->new_pg_temp[p->first].clear(); - continue; - } + vector raw_up; + int primary; + tmpmap.pg_to_raw_up(p->first, &raw_up, &primary); + if (raw_up == p->second) { + ldout(cct, 10) << __func__ << " removing pg_temp " << p->first << " " + << p->second << " that matches raw_up mapping" << dendl; + if (osdmap.pg_temp->count(p->first)) + pending_inc->new_pg_temp[p->first].clear(); + else + pending_inc->new_pg_temp.erase(p->first); } } - map::iterator p = tmpmap.primary_temp->begin(); - while (p != tmpmap.primary_temp->end()) { + for (map::iterator p = tmpmap.primary_temp->begin(); + p != tmpmap.primary_temp->end(); + ++p) { // primary down? if (tmpmap.is_down(p->second)) { ldout(cct, 10) << __func__ << " removing primary_temp " << p->first << " to down " << p->second << dendl; pending_inc->new_primary_temp[p->first] = -1; - ++p; continue; } // redundant primary_temp? - if (pending_inc->new_primary_temp.count(p->first) == 0) { - vector real_up, templess_up; - int real_primary, templess_primary; - pg_t pgid = p->first; - tmpmap.pg_to_acting_osds(pgid, &real_up, &real_primary); - tmpmap.primary_temp->erase(p++); - tmpmap.pg_to_acting_osds(pgid, &templess_up, &templess_primary); - if (real_primary == templess_primary){ - ldout(cct, 10) << __func__ << " removing primary_temp " - << pgid << " -> " << real_primary - << " (unnecessary/redundant)" << dendl; + vector real_up, templess_up; + int real_primary, templess_primary; + pg_t pgid = p->first; + tmpmap.pg_to_acting_osds(pgid, &real_up, &real_primary); + tmpmap.pg_to_raw_up(pgid, &templess_up, &templess_primary); + if (real_primary == templess_primary){ + ldout(cct, 10) << __func__ << " removing primary_temp " + << pgid << " -> " << real_primary + << " (unnecessary/redundant)" << dendl; + if (osdmap.primary_temp->count(pgid)) pending_inc->new_primary_temp[pgid] = -1; - } - continue; // we incremented p above + else + pending_inc->new_primary_temp.erase(pgid); } - ++p; } } diff --git a/src/test/osd/TestOSDMap.cc b/src/test/osd/TestOSDMap.cc index 7723cb620687b..3410fe7ccbd16 100644 --- a/src/test/osd/TestOSDMap.cc +++ b/src/test/osd/TestOSDMap.cc @@ -253,26 +253,37 @@ TEST_F(OSDMapTest, PrimaryTempRespected) { TEST_F(OSDMapTest, CleanTemps) { set_up_map(); - pg_t rawpg(0, 0, -1); - pg_t pgid = osdmap.raw_pg_to_pg(rawpg); - vector up_osds, acting_osds; - int up_primary, acting_primary; - - osdmap.pg_to_up_acting_osds(pgid, &up_osds, &up_primary, - &acting_osds, &acting_primary); - - // stick calculated values in to temps OSDMap::Incremental pgtemp_map(osdmap.get_epoch() + 1); - pgtemp_map.new_pg_temp[pgid] = up_osds; - pgtemp_map.new_primary_temp[pgid] = up_primary; + OSDMap::Incremental pending_inc(osdmap.get_epoch() + 2); + pg_t pga = osdmap.raw_pg_to_pg(pg_t(0, 0)); + { + vector up_osds, acting_osds; + int up_primary, acting_primary; + osdmap.pg_to_up_acting_osds(pga, &up_osds, &up_primary, + &acting_osds, &acting_primary); + pgtemp_map.new_pg_temp[pga] = up_osds; + pgtemp_map.new_primary_temp[pga] = up_primary; + } + pg_t pgb = osdmap.raw_pg_to_pg(pg_t(1, 0)); + { + vector up_osds, acting_osds; + int up_primary, acting_primary; + osdmap.pg_to_up_acting_osds(pgb, &up_osds, &up_primary, + &acting_osds, &acting_primary); + pending_inc.new_pg_temp[pgb] = up_osds; + pending_inc.new_primary_temp[pgb] = up_primary; + } + osdmap.apply_incremental(pgtemp_map); - OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1); OSDMap::clean_temps(g_ceph_context, osdmap, &pending_inc); - EXPECT_TRUE(pending_inc.new_pg_temp.count(pgid) && - pending_inc.new_pg_temp[pgid].size() == 0); - EXPECT_EQ(-1, pending_inc.new_primary_temp[pgid]); + EXPECT_TRUE(pending_inc.new_pg_temp.count(pga) && + pending_inc.new_pg_temp[pga].size() == 0); + EXPECT_EQ(-1, pending_inc.new_primary_temp[pga]); + + EXPECT_TRUE(!pending_inc.new_pg_temp.count(pgb) && + !pending_inc.new_primary_temp.count(pgb)); } TEST_F(OSDMapTest, KeepsNecessaryTemps) { -- 2.39.5