From 5a722b7ddc06576f350c134939d76279bba3896b Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Wed, 19 Dec 2018 17:01:23 +0800 Subject: [PATCH] osd/OSDMap: cancel mapping if target osd is out Suppose we have a bad pg_upmap_item, say: ``` pg_upmap_items 2.0 [0,8,5,4,6,12] ``` which maps osd.6 to osd.12 that is currently marked as out. It turns out **maybe_remove_pg_upmaps** can not handle the above case well because **_apply_upmap** will silently discard any bad mappings whenver they try to target at some current __out__ OSDs. So if you call **pg_to_raw_up(2.0)**, you'll probably get something like: ```up [8,4,6] ``` (e.g., the last mapping pair 6->12 is simply ignored by **_apply_upmap**). Make **clean_pg_upmaps** do the tidy-up check instead, since it already has __bare__ access to those pg_upmaps and pg_upmap_items. Fixes: http://tracker.ceph.com/issues/37501 Signed-off-by: xie xingguo Signed-off-by: ningtao (cherry picked from commit d06b65a0ab210e7a920761765531d929dd5798ec) Conflicts: - *maybe_remove_pg_upmaps* input changed, in master we now have a passed in *nextmap* parameter --- src/osd/OSDMap.cc | 11 ++++++-- src/test/osd/TestOSDMap.cc | 52 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index fa9937b3f7562..65c7ea6295e41 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -4074,9 +4074,16 @@ int OSDMap::clean_pg_upmaps( pg_to_raw_osds(p.first, &raw, &primary); mempool::osdmap::vector> newmap; for (auto& q : p.second) { - if (std::find(raw.begin(), raw.end(), q.first) != raw.end()) { - newmap.push_back(q); + if (std::find(raw.begin(), raw.end(), q.first) == raw.end()) { + // cancel mapping if source osd does not exist anymore + continue; + } + if (q.second != CRUSH_ITEM_NONE && q.second < max_osd && + q.second >= 0 && osd_weight[q.second] == 0) { + // cancel mapping if target osd is out + continue; } + newmap.push_back(q); } if (newmap.empty()) { ldout(cct, 10) << " removing no-op pg_upmap_items " << p.first << " " diff --git a/src/test/osd/TestOSDMap.cc b/src/test/osd/TestOSDMap.cc index ec732b19ed75b..bb504a3c38855 100644 --- a/src/test/osd/TestOSDMap.cc +++ b/src/test/osd/TestOSDMap.cc @@ -679,6 +679,58 @@ TEST_F(OSDMapTest, CleanPGUpmaps) { } } + { + // http://tracker.ceph.com/issues/37501 + pg_t ec_pg(0, my_ec_pool); + pg_t ec_pgid = osdmap.raw_pg_to_pg(ec_pg); + OSDMap tmpmap; // use a tmpmap here, so we do not dirty origin map.. + int from = -1; + int to = -1; + { + // insert a valid pg_upmap_item + vector ec_up; + int ec_up_primary; + osdmap.pg_to_raw_up(ec_pgid, &ec_up, &ec_up_primary); + ASSERT_TRUE(!ec_up.empty()); + from = *(ec_up.begin()); + ASSERT_TRUE(from >= 0); + for (int i = 0; i < (int)get_num_osds(); i++) { + if (std::find(ec_up.begin(), ec_up.end(), i) == ec_up.end()) { + to = i; + break; + } + } + ASSERT_TRUE(to >= 0); + ASSERT_TRUE(from != to); + vector> new_pg_upmap_items; + new_pg_upmap_items.push_back(make_pair(from, to)); + OSDMap::Incremental pending_inc(osdmap.get_epoch() + 1); + pending_inc.new_pg_upmap_items[ec_pgid] = + mempool::osdmap::vector>( + new_pg_upmap_items.begin(), new_pg_upmap_items.end()); + tmpmap.deepish_copy_from(osdmap); + tmpmap.apply_incremental(pending_inc); + ASSERT_TRUE(tmpmap.have_pg_upmaps(ec_pgid)); + } + { + // mark one of the target OSDs of the above pg_upmap_item as out + OSDMap::Incremental pending_inc(tmpmap.get_epoch() + 1); + pending_inc.new_weight[to] = CEPH_OSD_OUT; + tmpmap.apply_incremental(pending_inc); + ASSERT_TRUE(tmpmap.is_out(to)); + ASSERT_TRUE(tmpmap.have_pg_upmaps(ec_pgid)); + } + { + // *maybe_remove_pg_upmaps* should be able to remove the above *bad* mapping + OSDMap::Incremental pending_inc(tmpmap.get_epoch() + 1); + OSDMap nextmap; + nextmap.deepish_copy_from(tmpmap); + nextmap.maybe_remove_pg_upmaps(g_ceph_context, nextmap, &pending_inc); + tmpmap.apply_incremental(pending_inc); + ASSERT_TRUE(!tmpmap.have_pg_upmaps(ec_pgid)); + } + } + { // TEST pg_upmap { -- 2.39.5