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: remove non-existent MDS daemons from FSMap #14937

Merged
merged 1 commit into from May 10, 2017

Conversation

Projects
None yet
5 participants
@SpandanKumarSahu
Contributor

SpandanKumarSahu commented May 3, 2017

Fixes: http://tracker.ceph.com/issues/17453
Signed-off-by: Spandan Kumar Sahu spandankumarsahu@gmail.com

@@ -521,6 +523,13 @@ void Mgr::handle_fs_map(MFSMap* m)
for (const auto &i : mds_info) {
const auto &info = i.second;
if(!new_fsmap.gid_exists(i.first)){

This comment has been minimized.

@liewegas

liewegas May 4, 2017

Member

"if ("

(missing a space)

@liewegas

This comment has been minimized.

Member

liewegas commented May 4, 2017

Otherwise this looks good!

continue;
}
//Remember which MDS exists so that we can cull any that don't

This comment has been minimized.

@tchaikov

tchaikov May 4, 2017

Contributor

please add a space after //

@SpandanKumarSahu

This comment has been minimized.

Contributor

SpandanKumarSahu commented May 5, 2017

Made the necessary changes.
@liewegas @tchaikov

}
// Remember which MDS exists so that we can cull any that don't
names_exist.insert(stringify(info.name));

This comment has been minimized.

@tchaikov

tchaikov May 5, 2017

Contributor

info.name is a string already, i don't think we need to stringify() it.

mgr: remove non-existent MDS daemons from FSMap
Fixes: http://tracker.ceph.com/issues/17453
Signed-off-by: Spandan Kumar Sahu <spandankumarsahu@gmail.com>
@SpandanKumarSahu

This comment has been minimized.

Contributor

SpandanKumarSahu commented May 5, 2017

@liewegas liewegas added the needs-qa label May 5, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented May 8, 2017

causes

2017-05-06 06:11:05.391477 7f26da7b0700 0 log_channel(cluster) log [INF] : HEALTH_WARN; insufficient standby daemons available: have 0; want 1 more

/a/sage-2017-05-06_05:54:44-rados-wip-sage-testing2---basic-smithi/1108765

@SpandanKumarSahu

This comment has been minimized.

Contributor

SpandanKumarSahu commented May 8, 2017

@liewegas Why is that so? Apparently, the code should remove only the ones which don't exist, right?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 9, 2017

i think this issue is unrelated to this change. instead, it is stemmed from tasks/cephfs/test_failover.py: test_grow_shrink(). in this test, we call

        fs_a.set_max_mds(3)

which set the max_mds to 3.

so all up mds were active after that

2017-05-06 06:11:04.602396 7f26da7b0700 10 mon.a@0(leader).mds e11 e11: 3/3/3 up {0=a=up:active,1=c=up:active,2=b=up:active}

in other words, none of the 3 MDS is standby anymore. but we set the standby count to 1 in MDSMap::check_health(). @batrick is this expected?

@batrick

This comment has been minimized.

Member

batrick commented May 10, 2017

Yes, that's expected. I will make a PR for test_grow_shrink that turns off that warning.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 10, 2017

@batrick thank you!

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 10, 2017

@liewegas as @batrick has confirmed this, can we merge this change?

@ukernel

This comment has been minimized.

Member

ukernel commented May 10, 2017

agree with tchaikov. the healthe warning is unrelated

@liewegas liewegas merged commit 9af1a1e into ceph:master May 10, 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
@liewegas

This comment has been minimized.

Member

liewegas commented May 10, 2017

thanks everyone!

@batrick

This comment has been minimized.

Member

batrick commented May 10, 2017

FYI: #15035

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment