Skip to content

Commit

Permalink
mon: paxos: introduce new reset_pending_committing_finishers for safety
Browse files Browse the repository at this point in the history
There are asserts about the state of the system and pending_finishers which can
be triggered by running arbitrary commands through again. They are correct
when not restarting, but when we do restart we need to take care to preserve
the same invariants as appropriate. Use this function to be careful about
the order of committing_finishers v pending_finishers and to make sure they're
both empty before any Contexts actually get called.

We also reorder a call to finish_contexts on the waiting_for_writeable list for
similar reasons.

Fixes: http://tracker.ceph.com/issues/39484

Signed-off-by: Greg Farnum <[email protected]>
  • Loading branch information
gregsfortytwo committed Apr 30, 2019
1 parent 8cd1128 commit b17caec
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
14 changes: 8 additions & 6 deletions src/mon/Paxos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,7 @@ void Paxos::leader_init()
// discard pending transaction
pending_proposal.reset();

finish_contexts(g_ceph_context, pending_finishers, -EAGAIN);
finish_contexts(g_ceph_context, committing_finishers, -EAGAIN);
reset_pending_committing_finishers();

logger->inc(l_paxos_start_leader);

Expand Down Expand Up @@ -1375,9 +1374,8 @@ void Paxos::peon_init()
pending_proposal.reset();

// no chance to write now!
reset_pending_committing_finishers();
finish_contexts(g_ceph_context, waiting_for_writeable, -EAGAIN);
finish_contexts(g_ceph_context, pending_finishers, -EAGAIN);
finish_contexts(g_ceph_context, committing_finishers, -EAGAIN);

logger->inc(l_paxos_start_peon);
}
Expand All @@ -1400,13 +1398,17 @@ void Paxos::restart()
// discard pending transaction
pending_proposal.reset();

finish_contexts(g_ceph_context, committing_finishers, -EAGAIN);
finish_contexts(g_ceph_context, pending_finishers, -EAGAIN);
reset_pending_committing_finishers();
finish_contexts(g_ceph_context, waiting_for_active, -EAGAIN);

logger->inc(l_paxos_restart);
}

void Paxos::reset_pending_committing_finishers()
{
committing_finishers.splice(committing_finishers.end(), pending_finishers);
finish_contexts(g_ceph_context, committing_finishers, -EAGAIN);
}

void Paxos::dispatch(MonOpRequestRef op)
{
Expand Down
9 changes: 9 additions & 0 deletions src/mon/Paxos.h
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,15 @@ class Paxos {
* this list. When it commits, these finishers are notified.
*/
list<Context*> committing_finishers;
/**
* This function re-triggers pending_ and committing_finishers
* safely, so as to maintain existing system invariants. In particular
* we maintain ordering by triggering committing before pending, and
* we clear out pending_finishers prior to any triggers so that
* we don't trigger asserts on them being empty. You should
* use it instead of sending -EAGAIN to them with finish_contexts.
*/
void reset_pending_committing_finishers();

/**
* @defgroup Paxos_h_sync_warns Synchronization warnings
Expand Down

0 comments on commit b17caec

Please sign in to comment.