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

mgr: fix metadata handling from old MDS daemons #14161

Merged
merged 1 commit into from Apr 28, 2017

Conversation

Projects
None yet
3 participants
@jcsp
Contributor

jcsp commented Mar 27, 2017

Completely untested try at fixing the FIXME mentioned in #13617 (comment)

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

@jcsp jcsp added bug fix mgr labels Mar 27, 2017

@jcsp jcsp requested a review from liewegas Mar 27, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 27, 2017

IIRC the crash was on line 501 (when deciding whether to update) so by itself this patch won't help. THis is intended to be used along with abde684?

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 15, 2017

Apologies for long delay.

This patch should work, because the case on line 501 only applies when there is already some metadata in the map, and this change inserts it whenever we put it into the map.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 15, 2017

Oh, but the metadata could also get inserted the other way via load_all_metadata, in which case it would be missing the addr field. So yes, this works in conjunction with the abde684 change -- this patch is still important to avoid repeatedly reloading the metadata for old daemons.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 15, 2017

Rebased

John Spray
mgr: fix metadata handling from old MDS daemons
In abde684 we started always fetching metadata on fsmap changes
for legacy daemons (i.e. those without addr fields) -- this works
but is very inefficient.

By injecting an 'addr' field based on the address in
the FSMap, if the field is missing, we can reload
metadata only on daemon restarts.

Signed-off-by: John Spray <john.spray@redhat.com>
@@ -94,6 +101,13 @@ class MetadataUpdate : public Context
json_spirit::mObject daemon_meta = json_result.get_obj();
// Apply any defaults
for (const auto &i : defaults) {

This comment has been minimized.

@tchaikov

tchaikov Apr 26, 2017

Contributor

daemon_meta.insert(defaults.begin(), defaults.end()); would suffice.

@liewegas liewegas merged commit 92b6227 into ceph:master Apr 28, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment