mon: simplify states

- make states mutually exclusive (an enum)
- rename locked -> updating_previous
- set state prior to begin() to simplify things a bit

Signed-off-by: Sage Weil <sage@inktank.com>
(cherry picked from commit ee34a219605d1943740fdae0d84cfb9020302dd6)

diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc
index 5ff6acb1ab77530b1acb3ae2c09b87785d74a2bc..1b43da955464e39071eee6107b5267b82303ff6f 100644 (file)
--- a/src/mon/Paxos.cc
+++ b/src/mon/Paxos.cc
@@ -431,7 +431,7 @@ void Paxos::handle_last(MMonPaxos *last)
       if (uncommitted_v == last_committed+1 &&
          uncommitted_value.length()) {
        dout(10) << "that's everyone.  begin on old learned value" << dendl;
-       state = STATE_LOCKED;
+       state = STATE_UPDATING_PREVIOUS;
        begin(uncommitted_value);
       } else {
        // active!
@@ -470,7 +470,7 @@ void Paxos::begin(bufferlist& v)
           << dendl;
 
   assert(mon->is_leader());
-  state |= STATE_UPDATING;
+  assert(is_updating() || is_updating_previous());
 
   // we must already have a majority for this to work.
   assert(mon->get_quorum().size() == 1 ||
@@ -599,7 +599,7 @@ void Paxos::handle_accept(MMonPaxos *accept)
   assert(accept->last_committed == last_committed ||   // not committed
         accept->last_committed == last_committed-1);  // committed
 
-  assert(is_updating());
+  assert(is_updating() || is_updating_previous());
   assert(accepted.count(from) == 0);
   accepted.insert(from);
   dout(10) << " now " << accepted << " have accepted" << dendl;
@@ -638,7 +638,7 @@ void Paxos::accept_timeout()
   dout(5) << "accept timeout, calling fresh election" << dendl;
   accept_timeout_event = 0;
   assert(mon->is_leader());
-  assert(is_updating());
+  assert(is_updating() || is_updating_previous());
   mon->bootstrap();
 }
 
@@ -1200,7 +1200,6 @@ bool Paxos::is_readable(version_t v)
   return 
     (mon->is_peon() || mon->is_leader()) &&
     (is_active() || is_updating()) &&
-    !is_locked() &&
     last_committed > 0 &&           // must have a value
     (mon->get_quorum().size() == 1 ||  // alone, or
      is_lease_valid()); // have lease
@@ -1265,7 +1264,7 @@ void Paxos::propose_queued()
   list_proposals(*_dout);
   *_dout << dendl;
 
-  state = 0;
+  state = STATE_UPDATING;
   begin(proposal->bl);
 }
 

diff --git a/src/mon/Paxos.h b/src/mon/Paxos.h
index ac3c0347804247f30b2e34972c31c5e6b2b676a0..4f1af82836e5f82a2f98ddc5a9e25a010f18c48b 100644 (file)
--- a/src/mon/Paxos.h
+++ b/src/mon/Paxos.h
@@ -163,20 +163,24 @@ public:
    * @defgroup Paxos_h_states States on which the leader/peon may be.
    * @{
    */
-  /**
-   * Leader/Peon is in Paxos' Recovery state
-   */
-  const static int STATE_RECOVERING = 0x01;
-  /**
-   * Leader/Peon is idle, and the Peon may or may not have a valid lease.
-   */
-  const static int STATE_ACTIVE     = 0x02;
-  /**
-   * Leader/Peon is updating to a new value.
-   */
-  const static int STATE_UPDATING   = 0x04;
-
-  const static int STATE_LOCKED     = 0x10;
+  enum {
+    /**
+     * Leader/Peon is in Paxos' Recovery state
+     */
+    STATE_RECOVERING,
+    /**
+     * Leader/Peon is idle, and the Peon may or may not have a valid lease.
+     */
+    STATE_ACTIVE,
+    /**
+     * Leader/Peon is updating to a new value.
+     */
+    STATE_UPDATING,
+    /*
+     * Leader proposing an old value
+     */
+    STATE_UPDATING_PREVIOUS,
+  };
 
   /**
    * Obtain state name from constant value.
@@ -188,23 +192,18 @@ public:
    * @return The state's name.
    */
   static const string get_statename(int s) {
-    stringstream ss;
-    if (s & STATE_RECOVERING) {
-      ss << "recovering";
-      assert(!(s & ~(STATE_RECOVERING|STATE_LOCKED)));
-    } else if (s & STATE_ACTIVE) {
-      ss << "active";
-      assert(s == STATE_ACTIVE);
-    } else if (s & STATE_UPDATING) {
-      ss << "updating";
-      assert(!(s & ~(STATE_UPDATING|STATE_LOCKED)));
-    } else {
-      assert(0 == "We shouldn't have gotten here!");
+    switch (s) {
+    case STATE_RECOVERING:
+      return "recovering";
+    case STATE_ACTIVE:
+      return "active";
+    case STATE_UPDATING:
+      return "updating";
+    case STATE_UPDATING_PREVIOUS:
+      return "updating-previous";
+    default:
+      return "UNKNOWN";
     }
-
-    if (s & STATE_LOCKED)
-      ss << " (locked)";
-    return ss.str();
   }
 
 private:
@@ -234,9 +233,13 @@ public:
    *
    * @return 'true' if we are on the Updating state; 'false' otherwise.
    */
-  bool is_updating() const { return (state & STATE_UPDATING); }
+  bool is_updating() const { return state == STATE_UPDATING; }
 
-  bool is_locked() const { return (state & STATE_LOCKED); }
+  /**
+   * Check if we are updating/proposing a previous value from a
+   * previous quorum
+   */
+  bool is_updating_previous() const { return state == STATE_UPDATING_PREVIOUS; }
 
 private:
   /**

diff --git a/src/mon/PaxosService.h b/src/mon/PaxosService.h
index c3d97ebb98cbe565bb1b40bd6c3a8e5aeee7e6a4..5398f20802174c5202dfbb45c6cc61ae290ffba1 100644 (file)
--- a/src/mon/PaxosService.h
+++ b/src/mon/PaxosService.h
@@ -516,8 +516,9 @@ public:
    * @returns true if in state ACTIVE; false otherwise.
    */
   bool is_active() {
-    return (!is_proposing() && !paxos->is_recovering()
-        && !paxos->is_locked());
+    return
+      !is_proposing() &&
+      (paxos->is_active() || paxos->is_updating() || paxos->is_updating_previous());
   }
 
   /**