Skip to content

Commit

Permalink
crush: add root_bucket to identify underfull buckets
Browse files Browse the repository at this point in the history
All underfull buckets under root_buckets will be taken as target

For the crule rule:
    step take datacenter0
    step chooseleaf firstn 2 type host
    step emit
    step take datacenter1
    step chooseleaf firstn 2 type host
    step emit

If one host contains overfull osd but no underfull osd,
it will use other underfull buckets as target, which
maybe not in the same datacenter, that will
broke the rule.

Fixes: http://tracker.ceph.com/issues/38826

Signed-off-by: huangjun <huangjun@xsky.com>
(cherry picked from commit 3d5678d)
  • Loading branch information
huangjun authored and xiexingguo committed Mar 28, 2019
1 parent 520f7c0 commit 5c173a0
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
16 changes: 11 additions & 5 deletions src/crush/CrushWrapper.cc
Expand Up @@ -3675,7 +3675,8 @@ int CrushWrapper::_choose_type_stack(
const vector<int>& orig,
vector<int>::const_iterator& i,
set<int>& used,
vector<int> *pw) const
vector<int> *pw,
int root_bucket) const
{
vector<int> w = *pw;
vector<int> o;
Expand All @@ -3685,7 +3686,7 @@ int CrushWrapper::_choose_type_stack(
<< " at " << *i
<< " pw " << *pw
<< dendl;

ceph_assert(root_bucket < 0);
vector<int> cumulative_fanout(stack.size());
int f = 1;
for (int j = (int)stack.size() - 1; j >= 0; --j) {
Expand Down Expand Up @@ -3713,6 +3714,10 @@ int CrushWrapper::_choose_type_stack(
item = get_parent_of_type(item, type);
ldout(cct, 10) << __func__ << " underfull " << osd << " type " << type
<< " is " << item << dendl;
if (!subtree_contains(root_bucket, item)) {
ldout(cct, 20) << __func__ << " not in root subtree " << root_bucket << dendl;
continue;
}
underfull_buckets[j].insert(item);
}
}
Expand Down Expand Up @@ -3872,7 +3877,7 @@ int CrushWrapper::try_remap_rule(
set<int> used;

vector<pair<int,int>> type_stack; // (type, fan-out)

int root_bucket = 0;
for (unsigned step = 0; step < rule->len; ++step) {
const crush_rule_step *curstep = &rule->steps[step];
ldout(cct, 10) << __func__ << " step " << step << " w " << w << dendl;
Expand All @@ -3883,6 +3888,7 @@ int CrushWrapper::try_remap_rule(
map->buckets[-1-curstep->arg1])) {
w.clear();
w.push_back(curstep->arg1);
root_bucket = curstep->arg1;
ldout(cct, 10) << __func__ << " take " << w << dendl;
} else {
ldout(cct, 1) << " bad take value " << curstep->arg1 << dendl;
Expand All @@ -3900,7 +3906,7 @@ int CrushWrapper::try_remap_rule(
if (type > 0)
type_stack.push_back(make_pair(0, 1));
int r = _choose_type_stack(cct, type_stack, overfull, underfull, orig,
i, used, &w);
i, used, &w, root_bucket);
if (r < 0)
return r;
type_stack.clear();
Expand All @@ -3922,7 +3928,7 @@ int CrushWrapper::try_remap_rule(
ldout(cct, 10) << " emit " << w << dendl;
if (!type_stack.empty()) {
int r = _choose_type_stack(cct, type_stack, overfull, underfull, orig,
i, used, &w);
i, used, &w, root_bucket);
if (r < 0)
return r;
type_stack.clear();
Expand Down
3 changes: 2 additions & 1 deletion src/crush/CrushWrapper.h
Expand Up @@ -1532,7 +1532,8 @@ class CrushWrapper {
const vector<int>& orig,
vector<int>::const_iterator& i,
set<int>& used,
vector<int> *pw) const;
vector<int> *pw,
int root_bucket) const;

int try_remap_rule(
CephContext *cct,
Expand Down

0 comments on commit 5c173a0

Please sign in to comment.