From fe03383e6691f6bd95189d3ba4d91fbb2ea9e9c8 Mon Sep 17 00:00:00 2001 From: David Zafman Date: Tue, 26 Nov 2019 15:19:01 -0800 Subject: [PATCH] osd: Create more_underfull with below target that aren't in underfull Use more_underfull for finding remaps for overfull OSDs Signed-off-by: David Zafman --- src/crush/CrushWrapper.cc | 37 +++++++++++++++++++++++++++++++--- src/crush/CrushWrapper.h | 2 ++ src/osd/OSDMap.cc | 21 ++++++++++++------- src/osd/OSDMap.h | 1 + src/test/crush/CrushWrapper.cc | 28 ++++++++++++++++++++----- 5 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index 4b90fec6217..0f0e3e2efef 100644 --- a/src/crush/CrushWrapper.cc +++ b/src/crush/CrushWrapper.cc @@ -3843,6 +3843,7 @@ int CrushWrapper::_choose_type_stack( const vector>& stack, const set& overfull, const vector& underfull, + const vector& more_underfull, const vector& orig, vector::const_iterator& i, set& used, @@ -3955,6 +3956,33 @@ int CrushWrapper::_choose_type_stack( ++i; break; } + if (!replaced) { + for (auto item : more_underfull) { + ldout(cct, 10) << __func__ << " more underfull pos " << pos + << " was " << *i << " considering " << item + << dendl; + if (used.count(item)) { + ldout(cct, 20) << __func__ << " in used " << used << dendl; + continue; + } + if (!subtree_contains(from, item)) { + ldout(cct, 20) << __func__ << " not in subtree " << from << dendl; + continue; + } + if (std::find(orig.begin(), orig.end(), item) != orig.end()) { + ldout(cct, 20) << __func__ << " in orig " << orig << dendl; + continue; + } + o.push_back(item); + used.insert(item); + ldout(cct, 10) << __func__ << " pos " << pos << " replace " + << *i << " -> " << item << dendl; + replaced = true; + assert(i != orig.end()); + ++i; + break; + } + } } if (!replaced) { ldout(cct, 10) << __func__ << " pos " << pos << " keep " << *i @@ -4032,6 +4060,7 @@ int CrushWrapper::try_remap_rule( int maxout, const set& overfull, const vector& underfull, + const vector& more_underfull, const vector& orig, vector *out) const { @@ -4041,7 +4070,9 @@ int CrushWrapper::try_remap_rule( ldout(cct, 10) << __func__ << " ruleno " << ruleno << " numrep " << maxout << " overfull " << overfull - << " underfull " << underfull << " orig " << orig + << " underfull " << underfull + << " more_underfull " << more_underfull + << " orig " << orig << dendl; vector w; // working set out->clear(); @@ -4078,7 +4109,7 @@ int CrushWrapper::try_remap_rule( type_stack.push_back(make_pair(type, numrep)); if (type > 0) type_stack.push_back(make_pair(0, 1)); - int r = _choose_type_stack(cct, type_stack, overfull, underfull, orig, + int r = _choose_type_stack(cct, type_stack, overfull, underfull, more_underfull, orig, i, used, &w, root_bucket, ruleno); if (r < 0) return r; @@ -4100,7 +4131,7 @@ int CrushWrapper::try_remap_rule( case CRUSH_RULE_EMIT: ldout(cct, 10) << " emit " << w << dendl; if (!type_stack.empty()) { - int r = _choose_type_stack(cct, type_stack, overfull, underfull, orig, + int r = _choose_type_stack(cct, type_stack, overfull, underfull, more_underfull, orig, i, used, &w, root_bucket, ruleno); if (r < 0) return r; diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h index 694d0fa561c..97970546038 100644 --- a/src/crush/CrushWrapper.h +++ b/src/crush/CrushWrapper.h @@ -1591,6 +1591,7 @@ public: const std::vector>& stack, const std::set& overfull, const std::vector& underfull, + const std::vector& more_underfull, const std::vector& orig, std::vector::const_iterator& i, std::set& used, @@ -1604,6 +1605,7 @@ public: int maxout, const std::set& overfull, const std::vector& underfull, + const std::vector& more_underfull, const std::vector& orig, std::vector *out) const; diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index 176f9919b90..150add76732 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -4465,6 +4465,7 @@ bool OSDMap::try_pg_upmap( pg_t pg, ///< pg to potentially remap const set& overfull, ///< osds we'd want to evacuate const vector& underfull, ///< osds to move to, in order of preference + const vector& more_underfull, ///< more osds only slightly underfull vector *orig, vector *out) ///< resulting alternative mapping { @@ -4493,6 +4494,7 @@ bool OSDMap::try_pg_upmap( rule, pool->get_size(), overfull, underfull, + more_underfull, *orig, out); if (r < 0) @@ -4611,6 +4613,7 @@ int OSDMap::calc_pg_upmaps( // build overfull and underfull set overfull; vector underfull; + vector more_underfull; for (auto i = deviation_osd.rbegin(); i != deviation_osd.rend(); i++) { ldout(cct, 30) << " check " << i->first << " <= " << max_deviation << dendl; if (i->first <= max_deviation) @@ -4625,13 +4628,17 @@ int OSDMap::calc_pg_upmaps( for (auto i = deviation_osd.begin(); i != deviation_osd.end(); i++) { ldout(cct, 30) << " check " << i->first << " >= " << -(int)max_deviation << dendl; - if (i->first >= -(int)max_deviation) + if (i->first >= 0) break; - ldout(cct, 30) << " add underfull osd." << i->second << dendl; - underfull.push_back(i->second); + if (i->first < -(int)max_deviation) { + ldout(cct, 30) << " add underfull osd." << i->second << dendl; + underfull.push_back(i->second); + } else { + more_underfull.push_back(i->second); + } } - if (underfull.empty()) { - ldout(cct, 20) << __func__ << " failed to build underfull" << dendl; + if (underfull.empty() && more_underfull.empty()) { + ldout(cct, 20) << __func__ << " failed to build underfull and more_underfull" << dendl; break; } @@ -4648,7 +4655,7 @@ int OSDMap::calc_pg_upmaps( auto temp_pgs_by_osd = pgs_by_osd; // always start with fullest, break if we find any changes to make for (auto p = deviation_osd.rbegin(); p != deviation_osd.rend(); ++p) { - if (skip_overfull) { + if (skip_overfull && !underfull.empty()) { ldout(cct, 10) << " skipping overfull " << dendl; break; // fall through to check underfull } @@ -4760,7 +4767,7 @@ int OSDMap::calc_pg_upmaps( ldout(cct, 10) << " trying " << pg << dendl; vector raw, orig, out; tmp.pg_to_raw_upmap(pg, &raw, &orig); // including existing upmaps too - if (!try_pg_upmap(cct, pg, overfull, underfull, &orig, &out)) { + if (!try_pg_upmap(cct, pg, overfull, underfull, more_underfull, &orig, &out)) { continue; } ldout(cct, 10) << " " << pg << " " << orig << " -> " << out << dendl; diff --git a/src/osd/OSDMap.h b/src/osd/OSDMap.h index 9eb75b865ad..ae5731b72db 100644 --- a/src/osd/OSDMap.h +++ b/src/osd/OSDMap.h @@ -1433,6 +1433,7 @@ public: pg_t pg, ///< pg to potentially remap const std::set& overfull, ///< osds we'd want to evacuate const std::vector& underfull, ///< osds to move to, in order of preference + const std::vector& more_underfull, ///< less full osds to move to, in order of preference std::vector *orig, std::vector *out); ///< resulting alternative mapping diff --git a/src/test/crush/CrushWrapper.cc b/src/test/crush/CrushWrapper.cc index 575af8a01dd..6cfdd3df16c 100644 --- a/src/test/crush/CrushWrapper.cc +++ b/src/test/crush/CrushWrapper.cc @@ -1329,9 +1329,10 @@ TEST(CrushWrapper, try_remap_rule) { vector orig = { 0, 3, 9 }; set overfull = { 3 }; vector underfull = { 0, 2, 5, 8, 11 }; + vector more_underfull = {}; vector out; int r = c.try_remap_rule(g_ceph_context, rule, 3, - overfull, underfull, + overfull, underfull, more_underfull, orig, &out); cout << orig << " -> r = " << (int)r << " out " << out << std::endl; ASSERT_EQ(r, 0); @@ -1345,7 +1346,7 @@ TEST(CrushWrapper, try_remap_rule) { orig = {1, 3, 9}; r = c.try_remap_rule(g_ceph_context, rule, 3, - overfull, underfull, + overfull, underfull, more_underfull, orig, &out); cout << orig << " -> r = " << (int)r << " out " << out << std::endl; ASSERT_EQ(r, 0); @@ -1353,6 +1354,21 @@ TEST(CrushWrapper, try_remap_rule) { ASSERT_EQ(1, out[0]); ASSERT_EQ(0, out[1]); ASSERT_EQ(9, out[2]); + // + // Check that more_underfull is used when underfull runs out + orig = { 0, 3, 9 }; + overfull = { 3, 9 }; + underfull = { 2 }; + more_underfull = { 5, 8, 11 }; + r = c.try_remap_rule(g_ceph_context, rule, 3, + overfull, underfull, more_underfull, + orig, &out); + cout << orig << " -> r = " << (int)r << " out " << out << std::endl; + ASSERT_EQ(r, 0); + ASSERT_EQ(3u, out.size()); + ASSERT_EQ(0, out[0]); + ASSERT_EQ(2, out[1]); + ASSERT_EQ(5, out[2]); } // chooseleaf @@ -1366,9 +1382,10 @@ TEST(CrushWrapper, try_remap_rule) { vector orig = { 0, 3, 9 }; set overfull = { 3 }; vector underfull = { 0, 2, 5, 8, 11 }; + vector more_underfull = { }; vector out; int r = c.try_remap_rule(g_ceph_context, rule, 3, - overfull, underfull, + overfull, underfull, more_underfull, orig, &out); cout << orig << " -> r = " << (int)r << " out " << out << std::endl; ASSERT_EQ(r, 0); @@ -1392,9 +1409,10 @@ TEST(CrushWrapper, try_remap_rule) { vector orig = { 0, 3, 16, 12 }; set overfull = { 3, 12 }; vector underfull = { 6, 7, 9, 3, 0, 1, 15, 16, 13, 2, 5, 8, 11 }; + vector more_underfull = { }; vector out; int r = c.try_remap_rule(g_ceph_context, rule, 3, - overfull, underfull, + overfull, underfull, more_underfull, orig, &out); cout << orig << " -> r = " << (int)r << " out " << out << std::endl; ASSERT_EQ(r, 0); @@ -1407,7 +1425,7 @@ TEST(CrushWrapper, try_remap_rule) { orig.pop_back(); out.clear(); r = c.try_remap_rule(g_ceph_context, rule, 3, - overfull, underfull, + overfull, underfull, more_underfull, orig, &out); cout << orig << " -> r = " << (int)r << " out " << out << std::endl; ASSERT_EQ(r, 0); -- 2.39.5