mon/Paxos: separate proposal commit from the end of the round

Each commit should match with exactly one proposal; finish it when we
actually commit it and make sensible asserts.

The old finish_proposal() turns into finish_round(), and performs
generic checks and cleanup associated with the transition from
updating -> active.

Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc
index 22235c99c435364676bb3264e37777dd71f43435..26886451a49462fbd743cf1bd7e3e67691191b10 100644 (file)
--- a/src/mon/Paxos.cc
+++ b/src/mon/Paxos.cc
@@ -428,7 +428,7 @@ void Paxos::handle_last(MMonPaxos *last)
        extend_lease();
 
        if (do_refresh()) {
-         finish_proposal();
+         finish_round();
 
          finish_contexts(g_ceph_context, waiting_for_active);
          finish_contexts(g_ceph_context, waiting_for_readable);
@@ -510,7 +510,9 @@ void Paxos::begin(bufferlist& v)
     // we're alone, take it easy
     commit();
     if (do_refresh()) {
-      finish_proposal();
+      assert(is_updating());  // we can't be updating-previous with quorum of 1
+      commit_proposal();
+      finish_round();
       finish_contexts(g_ceph_context, waiting_for_active);
       finish_contexts(g_ceph_context, waiting_for_commit);
       finish_contexts(g_ceph_context, waiting_for_readable);
@@ -616,6 +618,9 @@ void Paxos::handle_accept(MMonPaxos *accept)
     commit();
     if (!do_refresh())
       goto out;
+    if (is_updating())
+      commit_proposal();
+    finish_contexts(g_ceph_context, waiting_for_commit);
   }
 
   // done?
@@ -628,11 +633,10 @@ void Paxos::handle_accept(MMonPaxos *accept)
     // yay!
     extend_lease();
 
-    finish_proposal();
+    finish_round();
 
     // wake people up
     finish_contexts(g_ceph_context, waiting_for_active);
-    finish_contexts(g_ceph_context, waiting_for_commit);
     finish_contexts(g_ceph_context, waiting_for_readable);
     finish_contexts(g_ceph_context, waiting_for_writeable);
   }
@@ -809,30 +813,28 @@ bool Paxos::do_refresh()
   return true;
 }
 
-void Paxos::finish_proposal()
+void Paxos::commit_proposal()
 {
+  dout(10) << __func__ << dendl;
   assert(mon->is_leader());
+  assert(!proposals.empty());
+  assert(is_updating());
 
-  // ok, now go active!
-  state = STATE_ACTIVE;
+  C_Proposal *proposal = static_cast<C_Proposal*>(proposals.front());
+  assert(proposal->proposed);
+  dout(10) << __func__ << " proposal " << proposal << " took "
+          << (ceph_clock_now(NULL) - proposal->proposal_time)
+          << " to finish" << dendl;
+  proposals.pop_front();
+  proposal->complete(0);
+}
 
-  // finish off the last proposal
-  if (!proposals.empty()) {
-    assert(mon->is_leader());
+void Paxos::finish_round()
+{
+  assert(mon->is_leader());
 
-    C_Proposal *proposal = static_cast<C_Proposal*>(proposals.front());
-    if (!proposal->proposed) {
-      dout(10) << __func__ << " proposal " << proposal << ": we must have received a stay message and we're "
-              << "trying to finish before time. "
-              << "Instead, propose it (if we are active)!" << dendl;
-    } else {
-      dout(10) << __func__ << " proposal " << proposal << " took "
-              << (ceph_clock_now(NULL) - proposal->proposal_time)
-              << " to finish" << dendl;
-      proposals.pop_front();
-      proposal->complete(0);
-    }
-  }
+  // ok, now go active!
+  state = STATE_ACTIVE;
 
   dout(10) << __func__ << " state " << state
           << " proposals left " << proposals.size() << dendl;

diff --git a/src/mon/Paxos.h b/src/mon/Paxos.h
index 5860aaac2abf43ddcd51e218cd6429acde708507..c85bcaf495e0e63bf6f9380a289a460f820abe16 100644 (file)
--- a/src/mon/Paxos.h
+++ b/src/mon/Paxos.h
@@ -994,7 +994,8 @@ private:
    */
   bool do_refresh();
 
-  void finish_proposal();
+  void commit_proposal();
+  void finish_round();
 
 public:
   /**