Skip to content

Commit

Permalink
Merge pull request #30898 from smithfarm/wip-42128-mimic
Browse files Browse the repository at this point in the history 
mimic: osd/OSDMap: do not trust partially simplified pg_upmap_item

Reviewed-by: xie xingguo <xie.xingguo@zte.com.cn>
  • Loading branch information
@yuriw
yuriw committed Oct 21, 2019
2 parents ec096ed + ecec556 commit 982284c
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 39 deletions.
80 changes: 41 additions & 39 deletions src/osd/OSDMap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1663,45 +1663,6 @@ bool OSDMap::check_pg_upmaps(
}
vector<int> raw, up;
pg_to_raw_upmap(pg, &raw, &up);
auto i = pg_upmap.find(pg);
if (i != pg_upmap.end() && raw == i->second) {
ldout(cct, 10) << " removing redundant pg_upmap "
<< i->first << " " << i->second
<< dendl;
to_cancel->push_back(pg);
continue;
}
auto j = pg_upmap_items.find(pg);
if (j != pg_upmap_items.end()) {
mempool::osdmap::vector<pair<int,int>> newmap;
for (auto& p : j->second) {
if (std::find(raw.begin(), raw.end(), p.first) == raw.end()) {
// cancel mapping if source osd does not exist anymore
continue;
}
if (p.second != CRUSH_ITEM_NONE && p.second < max_osd &&
p.second >= 0 && osd_weight[p.second] == 0) {
// cancel mapping if target osd is out
continue;
}
newmap.push_back(p);
}
if (newmap.empty()) {
ldout(cct, 10) << " removing no-op pg_upmap_items "
<< j->first << " " << j->second
<< dendl;
to_cancel->push_back(pg);
continue;
} else if (newmap != j->second) {
ldout(cct, 10) << " simplifying partially no-op pg_upmap_items "
<< j->first << " " << j->second
<< " -> " << newmap
<< dendl;
to_remap->insert({pg, newmap});
any_change = true;
continue;
}
}
auto crush_rule = get_pg_pool_crush_rule(pg);
auto r = crush->verify_upmap(cct,
crush_rule,
Expand Down Expand Up @@ -1746,6 +1707,47 @@ bool OSDMap::check_pg_upmaps(
break;
}
}
if (!to_cancel->empty() && to_cancel->back() == pg)
continue;
// okay, upmap is valid
// continue to check if it is still necessary
auto i = pg_upmap.find(pg);
if (i != pg_upmap.end() && raw == i->second) {
ldout(cct, 10) << " removing redundant pg_upmap "
<< i->first << " " << i->second
<< dendl;
to_cancel->push_back(pg);
continue;
}
auto j = pg_upmap_items.find(pg);
if (j != pg_upmap_items.end()) {
mempool::osdmap::vector<pair<int,int>> newmap;
for (auto& p : j->second) {
if (std::find(raw.begin(), raw.end(), p.first) == raw.end()) {
// cancel mapping if source osd does not exist anymore
continue;
}
if (p.second != CRUSH_ITEM_NONE && p.second < max_osd &&
p.second >= 0 && osd_weight[p.second] == 0) {
// cancel mapping if target osd is out
continue;
}
newmap.push_back(p);
}
if (newmap.empty()) {
ldout(cct, 10) << " removing no-op pg_upmap_items "
<< j->first << " " << j->second
<< dendl;
to_cancel->push_back(pg);
} else if (newmap != j->second) {
ldout(cct, 10) << " simplifying partially no-op pg_upmap_items "
<< j->first << " " << j->second
<< " -> " << newmap
<< dendl;
to_remap->insert({pg, newmap});
any_change = true;
}
}
}
any_change = any_change || !to_cancel->empty();
return any_change;
Expand Down
86 changes: 86 additions & 0 deletions src/test/osd/TestOSDMap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,92 @@ TEST_F(OSDMapTest, BUG_40104) {
}
}

TEST_F(OSDMapTest, BUG_42052) {
// https://tracker.ceph.com/issues/42052
set_up_map(6, true);
const string pool_name("pool");
// build customized crush rule for "pool"
CrushWrapper crush;
get_crush(osdmap, crush);
string rule_name = "rule";
int rule_type = pg_pool_t::TYPE_REPLICATED;
ASSERT_TRUE(!crush.rule_exists(rule_name));
int rno;
for (rno = 0; rno < crush.get_max_rules(); rno++) {
if (!crush.rule_exists(rno) && !crush.ruleset_exists(rno))
break;
}
int min_size = 3;
int max_size = 3;
int steps = 8;
crush_rule *rule = crush_make_rule(steps, rno, rule_type, min_size, max_size);
int step = 0;
crush_rule_set_step(rule, step++, CRUSH_RULE_SET_CHOOSELEAF_TRIES, 5, 0);
crush_rule_set_step(rule, step++, CRUSH_RULE_SET_CHOOSE_TRIES, 100, 0);
// always choose osd.0, osd.1, osd.2
crush_rule_set_step(rule, step++, CRUSH_RULE_TAKE, 0, 0);
crush_rule_set_step(rule, step++, CRUSH_RULE_EMIT, 0, 0);
crush_rule_set_step(rule, step++, CRUSH_RULE_TAKE, 0, 1);
crush_rule_set_step(rule, step++, CRUSH_RULE_EMIT, 0, 0);
crush_rule_set_step(rule, step++, CRUSH_RULE_TAKE, 0, 2);
crush_rule_set_step(rule, step++, CRUSH_RULE_EMIT, 0, 0);
ASSERT_TRUE(step == steps);
auto r = crush_add_rule(crush.get_crush_map(), rule, rno);
ASSERT_TRUE(r >= 0);
crush.set_rule_name(rno, rule_name);
{
OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1);
pending_inc.crush.clear();
crush.encode(pending_inc.crush, CEPH_FEATURES_SUPPORTED_DEFAULT);
osdmap.apply_incremental(pending_inc);
}

// create "pool"
OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1);
pending_inc.new_pool_max = osdmap.get_pool_max();
auto pool_id = ++pending_inc.new_pool_max;
pg_pool_t empty;
auto p = pending_inc.get_new_pool(pool_id, &empty);
p->size = 3;
p->min_size = 1;
p->set_pg_num(1);
p->set_pgp_num(1);
p->type = pg_pool_t::TYPE_REPLICATED;
p->crush_rule = rno;
p->set_flag(pg_pool_t::FLAG_HASHPSPOOL);
pending_inc.new_pool_names[pool_id] = pool_name;
osdmap.apply_incremental(pending_inc);
ASSERT_TRUE(osdmap.have_pg_pool(pool_id));
ASSERT_TRUE(osdmap.get_pool_name(pool_id) == pool_name);
pg_t rawpg(0, pool_id);
pg_t pgid = osdmap.raw_pg_to_pg(rawpg);
{
// pg_upmap 1.0 [2,3,5]
vector<int32_t> new_up{2,3,5};
OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1);
pending_inc.new_pg_upmap[pgid] = mempool::osdmap::vector<int32_t>(
new_up.begin(), new_up.end());
osdmap.apply_incremental(pending_inc);
}
{
// pg_upmap_items 1.0 [0,3,4,5]
vector<pair<int32_t,int32_t>> new_pg_upmap_items;
new_pg_upmap_items.push_back(make_pair(0, 3));
new_pg_upmap_items.push_back(make_pair(4, 5));
OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1);
pending_inc.new_pg_upmap_items[pgid] =
mempool::osdmap::vector<pair<int32_t,int32_t>>(
new_pg_upmap_items.begin(), new_pg_upmap_items.end());
osdmap.apply_incremental(pending_inc);
}
{
OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1);
clean_pg_upmaps(g_ceph_context, osdmap, pending_inc);
osdmap.apply_incremental(pending_inc);
ASSERT_FALSE(osdmap.have_pg_upmaps(pgid));
}
}

TEST(PGTempMap, basic)
{
PGTempMap m;
Expand Down

0 comments on commit 982284c

Please sign in to comment.