mon: paxos: introduce new reset_pending_committing_finishers for safety

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 <gfarnum@redhat.com>

diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc
index 5d07457fdbdecb5d8a331a914c8f3e6970b1c50c..3a691931c9f2a2bc33f56144072671efa21af2cc 100644 (file)
--- a/src/mon/Paxos.cc
+++ b/src/mon/Paxos.cc
@@ -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);
 
@@ -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);
 }
@@ -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)
 {

diff --git a/src/mon/Paxos.h b/src/mon/Paxos.h
index 2b2e71d94a131b6bc6f07ba0d64cfb374cd58070..590279ed9cef07780ce9d1ac38528fd0445ebc3e 100644 (file)
--- a/src/mon/Paxos.h
+++ b/src/mon/Paxos.h
@@ -597,6 +597,15 @@ private:
    * 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