mon/Paxos: fix another uncommitted value corner case

It is possible that we begin the paxos recovery with an uncommitted
value for, say, commit 100.  During last/collect we discover 100 has been
committed already.  But also, another node provides an uncommitted value
for 101 with the same pn.  Currently, we refuse to learn it, because the
pn is not strictly > than our current uncommitted pn... even though it is
the next last_committed+1 value that we need.

There are two possible fixes here:

 - make this a >= as we can accept newer values from the same pn.
 - discard our uncommitted value metadata when we commit the value.

Let's do both!

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

diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc
index 6ba09dad8380eea1a9dc6d46d28633be27152ae9..4c940a41221a07a673afe5b07866e3af14ddbcbb 100644 (file)
--- a/src/mon/Paxos.cc
+++ b/src/mon/Paxos.cc
@@ -341,6 +341,15 @@ void Paxos::store_state(MMonPaxos *m)
       // apply.
       decode_append_transaction(t, it->second);
     }
+
+    // discard obsolete uncommitted value?
+    if (uncommitted_v && uncommitted_v <= last_committed) {
+      dout(10) << " forgetting obsolete uncommitted value " << uncommitted_v
+              << " pn " << uncommitted_pn << dendl;
+      uncommitted_v = 0;
+      uncommitted_pn = 0;
+      uncommitted_value.clear();
+    }
   }
   if (!t.empty()) {
     dout(30) << __func__ << " transaction dump:\n";
@@ -416,7 +425,7 @@ void Paxos::handle_last(MMonPaxos *last)
 
     // did this person send back an accepted but uncommitted value?
     if (last->uncommitted_pn) {
-      if (last->uncommitted_pn > uncommitted_pn &&
+      if (last->uncommitted_pn >= uncommitted_pn &&
          last->last_committed >= last_committed &&
          last->last_committed + 1 >= uncommitted_v) {
        uncommitted_v = last->last_committed+1;