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

mon: PaxosService: call post_refresh() instead of post_paxos_update() #5358

Merged
merged 1 commit into from Sep 7, 2015

Conversation

Projects
None yet
3 participants
@jecluis
Copy link
Member

commented Jul 27, 2015

mon: PaxosService: call post_refresh() instead of post_paxos_update()
Whenever the monitor finishes committing a proposal, we call
Monitor::refresh_from_paxos() to nudge the services to refresh.  Once
all services have refreshed, we would then call each services
post_paxos_update().

However, due to an unfortunate, non-critical bug, some services (mainly
the LogMonitor) could have messages pending in their
'waiting_for_finished_proposal' callback queue [1], and we need to nudge
those callbacks.

This patch adds a new step during the refresh phase: instead of calling
directly the service's post_paxos_update(), we introduce a
PaxosService::post_refresh() which will call the services
post_paxos_update() function first and then nudge those callbacks when
appropriate.

[1] - Given the monitor will send MLog messages to itself, and given the
service is not readable before its initial state is proposed and
committed, some of the initial MLog's would be stuck waiting for the
proposal to finish.  However, by design, we only nudge those message's
callbacks when an election finishes or, if the leader, when the proposal
finishes.  On peons, however, we would only nudge those callbacks if an
election happened to be triggered, hence the need for an alternate path
to retry any message waiting for the initial proposal to finish.

Fixes: #11470

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
(cherry picked from commit 1551ebb)

@jecluis jecluis added this to the firefly milestone Jul 27, 2015

@ghost ghost removed the needs-qa label Jul 28, 2015

@ghost ghost self-assigned this Jul 28, 2015

@smithfarm

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2015

@jecluis This has passed a rados suite (see http://tracker.ceph.com/issues/11644#rados for details). OK to merge, do you think? (I could not determine who merged the original commit 1551ebb so I'm asking you. . .)

@smithfarm smithfarm assigned jecluis and unassigned ghost Sep 7, 2015

@jecluis

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2015

@smithfarm I merged that one manually. I believe this patch is okay to be merged.

smithfarm added a commit that referenced this pull request Sep 7, 2015

Merge pull request #5358 from ceph/wip-11470.firefly
mon: PaxosService: call post_refresh() instead of post_paxos_update()

Reviewed-by: Joao Eduardo Luis <joao@suse.de>

@smithfarm smithfarm merged commit 6762295 into firefly Sep 7, 2015

@smithfarm smithfarm deleted the wip-11470.firefly branch Sep 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.