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

mds: fix FSMap upgrade with daemons in the map #8073

Merged
merged 3 commits into from Mar 16, 2016

Conversation

jcsp
Copy link
Contributor

@jcsp jcsp commented Mar 12, 2016

This would trigger a sanity() assertion because
you'd have standbys in a Filesystem::mds_map, instead
of in standby_daemons.

Also, fix handling the enabled flag.

Signed-off-by: John Spray john.spray@redhat.com

This would trigger a sanity() assertion because
you'd have standbys in a Filesystem::mds_map, instead
of in standby_daemons.

Also, fix handling the enabled flag.

Signed-off-by: John Spray <john.spray@redhat.com>
jcsp referenced this pull request in gregsfortytwo/ceph Mar 12, 2016
…ent FS

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
The upgrade path was asserting that FSMap::filesystems
was empty.  This would be violated if FSMap was initially
loaded from disk in old format, and then also received
off the network (but with a newer epoch) in old format.

Signed-off-by: John Spray <john.spray@redhat.com>
@jcsp jcsp changed the title [DNM yet, testing] mds: fix FSMap upgrade with daemons in the map mds: fix FSMap upgrade with daemons in the map Mar 12, 2016
@jcsp jcsp added bug-fix cephfs Ceph File System labels Mar 12, 2016
@jcsp
Copy link
Contributor Author

jcsp commented Mar 12, 2016

Tested this on my home cluster while upgrading, worked there.

@jcsp
Copy link
Contributor Author

jcsp commented Mar 14, 2016

@liewegas @gregsfortytwo

@liewegas liewegas added this to the jewel milestone Mar 14, 2016
@bniver
Copy link
Contributor

bniver commented Mar 14, 2016

what is your home cluster?

On Sat, Mar 12, 2016 at 3:44 PM, John Spray notifications@github.com
wrote:

Tested this on my home cluster while upgrading, worked there.


Reply to this email directly or view it on GitHub
#8073 (comment).

@jcsp
Copy link
Contributor Author

jcsp commented Mar 14, 2016

@smithfarm
Copy link
Contributor

@jcsp: Nice little cluster!

@gregsfortytwo
Copy link
Member

edit: n/m, wrong PR

@gregsfortytwo
Copy link
Member

@jcsp this looks better than my simple upgrade switch, but we also need gregsfortytwo@09fce2d or similar to make "fs new" an idempotent command. We can throw that in here if the patch looks good to you. :)

…ation

If we check for creating an FS with one already existing first, we will
reject (with EINVAL) duplicate attempts to create the same FS with
the same params. That makes it non-idempotent, which is bad!

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
@jcsp
Copy link
Contributor Author

jcsp commented Mar 15, 2016

Rebased and cherry-picked in greg's idempotency fix

@gregsfortytwo
Copy link
Member

I had the pre-rebase version in the (all-green!) http://pulpito.ceph.com/gregf-2016-03-15_04:07:43-fs-greg-fs-testing-314---basic-mira/

And in upgrade testing with ceph/ceph-qa-suite#875 it seems to be doing as well as current master (http://pulpito.ceph.com/gregf-2016-03-15_12:49:59-upgrade:infernalis-x-greg-fs-testing-314---basic-mira).

gregsfortytwo added a commit that referenced this pull request Mar 16, 2016
mds: fix FSMap upgrade with daemons in the map

Reviewed-by: Greg Farnum <gfarnum@redhat.com>
@gregsfortytwo gregsfortytwo merged commit b4f34bc into jewel Mar 16, 2016
@gregsfortytwo gregsfortytwo deleted the wip-fsmap-upgrade-jewel branch March 16, 2016 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants