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

Patch up the CRUSH map compatibility guards #2072

Merged
merged 2 commits into from Jul 5, 2014
Merged

Patch up the CRUSH map compatibility guards #2072

merged 2 commits into from Jul 5, 2014

Conversation

gregsfortytwo
Copy link
Member

The "ceph osd crush tunables optimal" and "ceph osd setcrushmap" commands left an incompatibility window. Close it by testing the proposed CRUSH change feature requirements against the existing cluster's feature bits.

We need to see if there's a feature which is not in the quorum_features,
not if there are no features in common!

Signed-off-by: Greg Farnum <greg@inktank.com>
…ster features

When we change the tunables, or set a new CRUSH map, we need to make sure it's
supported by all the monitors and OSDs currently participating in the cluster.

Fixes: #8738

Signed-off-by: Greg Farnum <greg@inktank.com>
@gregsfortytwo
Copy link
Member Author

I ran local tests on this with vstart, verifying that it protects changes while there are old daemons, and allows changes when the daemons are updated. Haven't done anything else with it.

@ghost
Copy link

ghost commented Jul 3, 2014

It would be nice to have a minimal test that shows the problem as it was and prove that it is gone after the patch.

@liewegas
Copy link
Member

liewegas commented Jul 3, 2014

Looks good to me!

@gregsfortytwo
Copy link
Member Author

@dachary, we don't have any of the mocks we'd need to do that reasonably, and as it stands testing this requires old and new daemons running in the same cluster to demonstrate.
I'd love for us to have enough mocking and utility classes set up to allow proper unit testing, but it's not going to happen as part of this series.


stringstream features_ss;

int r = check_cluster_features(features, features_ss);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if r == EAGAIN ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we won't apply the change, and the user will get output saying a pending OSD doesn't support it.

@ghost
Copy link

ghost commented Jul 3, 2014

@gregsfortytwo I'm asking because I've recently discovered the appeal of the upgrade suite ;-) It's non trivial work however, that's true.

@jecluis
Copy link
Member

jecluis commented Jul 4, 2014

the patches look good to me.

1 similar comment
@ghost
Copy link

ghost commented Jul 4, 2014

the patches look good to me.

liewegas pushed a commit that referenced this pull request Jul 5, 2014
Patch up the CRUSH map compatibility guards

Reviewed-by: Loic Dachary <loic@dachary.org>
Reviewed-by: Joao Eduardo Luis <joao.luis@inktank.com>
Reviewed-by: Sage Weil <sage@redhat.com>
@liewegas liewegas merged commit 50a2183 into next Jul 5, 2014
@liewegas liewegas deleted the wip-8738-next branch July 5, 2014 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants