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: do the shutdown in the right order #14952

Merged
merged 2 commits into from May 9, 2017

Conversation

Projects
None yet
2 participants
@tchaikov
Contributor

tchaikov commented May 4, 2017

@tchaikov tchaikov requested a review from liewegas May 4, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented May 4, 2017

I think the addresses the case where Mgr::init deadlocks with MgrStandby::shutdown, but it doesn't address teh more general/common case of MgrStandby::handle_mgr_map() doing a synchronous active_mgr->shutdown() (and blocking the dispatch thread) while Mgr::init() is still running and waiting for messages from the mon.

I think the options are

  1. make Mgr init etc state-driven so that it doesn't block and we can kill it at any time.
  2. give Mgr its own messenger and dispatch thread
  3. make MgrStandby have needs_stop, need_start etc states and make Mgr shutdown also asynchronous.

?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 4, 2017

the state-driven approach looks like a better way to fix these racing issues. i think we can have this fix, and in the meanwhile, pull together a fix to make mgr state-driven.

@liewegas

This comment has been minimized.

Member

liewegas commented May 5, 2017

Okay, that works for me. Have you run this through the qa suite yet? I think that is probably the test of whether it'll be a viable interim solution

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 5, 2017

@liewegas will include it in my next test batch.

tchaikov added some commits May 4, 2017

mgr: do not use pointers for MgrStandby members
it's simpler and easier to manage the life cycle of them this way.

Signed-off-by: Kefu Chai <kchai@redhat.com>
mgr: do the shutdown in the right order
Fixes: http://tracker.ceph.com/issues/19813
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 9, 2017

@liewegas the PR passed the rados qa run, can we merge it?

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

@tchaikov tchaikov deleted the tchaikov:wip-19813 branch May 9, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 9, 2017

will keep http://tracker.ceph.com/issues/19813 open until we have a complete solution.

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