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

jewel: mon: crash on shutdown, lease_ack_timeout event #15083

Merged
merged 7 commits into from Aug 24, 2017

Conversation

Projects
None yet
5 participants

@tchaikov tchaikov added this to the jewel milestone May 15, 2017

@asheplyakov asheplyakov requested a review from tchaikov May 15, 2017

@liewegas liewegas added the bug fix label May 15, 2017

@@ -4741,7 +4757,9 @@ void Monitor::scrub_event_start()
return;
}
scrub_event = new C_Scrub(this);
scrub_event = new C_MonContext(this, [this](int) {

This comment has been minimized.

@tchaikov

tchaikov May 16, 2017

Contributor

might want to remove C_Scrub and C_ScrubTimeout in the header? i.e. we might need to backport #10513 also?

@tchaikov

tchaikov May 16, 2017

Contributor

might want to remove C_Scrub and C_ScrubTimeout in the header? i.e. we might need to backport #10513 also?

@tchaikov

apart from the nit, lgtm.

stiopaa1 and others added some commits Jul 30, 2016

mon/Monitor: move C_Scrub, C_ScrubTimeout to .cc
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
(cherry picked from commit e587501)
mon/Elector:move C_ElectionExpire class to cc file
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
(cherry picked from commit c819596)
mon/Paxos: move classes to .cc file
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
(cherry picked from commit d21357a)
mon/PaxosService: move classes to cc file
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
(cherry picked from commit a4e979a)
Alexey Sheplyakov
jewel: mon: add override annotation to callback classes
The only purpose of this patch is to avoid merge conflicts while
cherry-picking commit 561cbde.
Alternatively one could cherry-pick 1effdfe,
but that one brings a lot of unrelated changes.

Signed-off-by: Alexey Sheplyakov <asheplyakov@mirantis.com>
mon/Elector: call cancel_timer() in shutdown()
instead of doing it manually.

Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 12139ae)
mon: check is_shutdown() in timer callbacks
introduce a helper class: C_MonContext, and initialize all timer events
using it, to ensure that they do check is_shutdown() before doing their
work.

Fixes: http://tracker.ceph.com/issues/19825
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 561cbde)

Conflicts:
	src/mon/MgrMonitor.cc: no such service in Jewel
@asheplyakov

This comment has been minimized.

Show comment
Hide comment
@asheplyakov

asheplyakov May 17, 2017

@tchaikov I've cherry-picked e587501 and a few related commits (which move the code from header files to *.cc ones)

asheplyakov commented May 17, 2017

@tchaikov I've cherry-picked e587501 and a few related commits (which move the code from header files to *.cc ones)

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov May 17, 2017

Contributor

@asheplyakov much better, thanks!

Contributor

tchaikov commented May 17, 2017

@asheplyakov much better, thanks!

@smithfarm smithfarm changed the title from jewel: mon crash on shutdown, lease_ack_timeout event to jewel: mon: crash on shutdown, lease_ack_timeout event Jun 19, 2017

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Jun 19, 2017

Contributor

Jenkins re-test this please

Contributor

smithfarm commented Jun 19, 2017

Jenkins re-test this please

@smithfarm smithfarm added the core label Jul 12, 2017

@smithfarm smithfarm requested a review from jdurgin Aug 23, 2017

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Aug 23, 2017

Contributor

@tchaikov @jdurgin This passed a rados suite with 1 failure ( a segfault - see http://tracker.ceph.com/issues/21063#note-2 ). Do you think it can be merged?

Edit:
@liewegas says that KStore is not supported, so I'm tentatively ruling the suite a "pass".

Contributor

smithfarm commented Aug 23, 2017

@tchaikov @jdurgin This passed a rados suite with 1 failure ( a segfault - see http://tracker.ceph.com/issues/21063#note-2 ). Do you think it can be merged?

Edit:
@liewegas says that KStore is not supported, so I'm tentatively ruling the suite a "pass".

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Aug 24, 2017

Contributor

@smithfarm i think this can be merged.

Contributor

tchaikov commented Aug 24, 2017

@smithfarm i think this can be merged.

@smithfarm smithfarm merged commit a07f4d3 into ceph:jewel Aug 24, 2017

6 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
Unmodified Submodules submodules for project are unmodified
Details
default Build finished.
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment