Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crush: misc changes/fixes for device classes #16805

Merged
merged 7 commits into from Aug 4, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Copy link
Member

liewegas commented Aug 4, 2017

No description provided.

tchaikov and others added some commits Jul 30, 2017

qa/workunits/mon/crush_ops.sh: remove existing dev class before setti…
…ng it

we cannot overwrite existing dev class, and "osd_class_update_on_start"
is true by default (see 0c885d6). so we should remove all device classes before
setting them.

Signed-off-by: Kefu Chai <kchai@redhat.com>
mon/OSDMonitor: kill unused variable 'ts'
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
test/osd-fast-mark-down: enable 'osd-class-update-on-start' by default
116cf75
will now hide all shadow trees(roots), so this is not applicable anymore
(actually it is misleading).

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
crush: make 'remove_root' idempotent
We might use 'crush link' to link same host into
different roots, which as a result can cause different
shadow trees reference same hosts too.

This means we may need to destory the same buckets(hosts, racks, etc.)
multiple times during rebuilding all shadow trees and hence 'remove_root'
shall be idempotent.

Fixes: http://tracker.ceph.com/issues/20845
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
crush: fix preservation of shadow bucket ids
1- a decompiled and recompiled was parsing the class bucket ids but it
wasn't actually using them.
2- rebuild_roots_with_classes() was throwing out the old ids and assigning
new ids when the tree was rebuilt.

Fix by passing in a (potentially partial) class_bucket map into
populate_classes().  Take care to allocate new bucket ids that don't
collide with previously used ids.

Signed-off-by: Sage Weil <sage@redhat.com>
crush/CrushWrapper: rebuild_roots_with_classes on bucket removal
Signed-off-by: Sage Weil <sage@redhat.com>
crush: remove cleanup_classes()
I can't for the life of me figure out what this is supposed to do.

- why remove (some) classes right after we populated them?
- why remove them after we decode the crush map?

Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas requested a review from xiexingguo Aug 4, 2017

@liewegas liewegas added this to the luminous milestone Aug 4, 2017

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Aug 4, 2017

This passes the crush_ops.sh locally. I'm a bit unsure though because I don't understand why cleanup_classes() was there before, or why the rebuild_roots_with_classes() was doing the last prune step...

@xiexingguo

This comment has been minimized.

Copy link
Member

xiexingguo commented Aug 4, 2017

This seems to be the simplest fix 👍

I'm a bit unsure though because I don't understand why cleanup_classes() was there before, or why the rebuild_roots_with_classes() was doing the last prune step...

My guess is:
We won't do any tidy-up works if a shadow root or tree becomes empty(e.g. all device classes are removed or all devices are purged, we instead rely oncleanup_classes/trim_roots_with_class(true)to do this) before this change. This can be problematic because we are unable to manually delete any shadow buckets.

    ./bin/ceph osd crush tree --show-shadow
    ID WEIGHT  TYPE NAME
    -6       0 root default~temp_class
    -5       0     host gitbuilder-ceph-rpm-centos7-amd64-basic~temp_class
    
    ./bin/ceph osd crush rm gitbuilder-ceph-rpm-centos7-amd64-basic~temp_class
    Invalid command:  invalid chars ~ in gitbuilder-ceph-rpm-centos7-amd64-basic~temp_class

But the above problem shall get fixed by the post commit 226d210 here since we now rebuild the shadow trees whenever a parent bucket or device is gone.

@liewegas liewegas merged commit 6dfae21 into ceph:master Aug 4, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@liewegas liewegas deleted the liewegas:wip-crush-classes branch Aug 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.