mon: fix store_state

We need to bound the range we write to disk by m->last_committed; this got
lost in translation in commit dfab2c91f53289b1d527fb87126a639f5d5275f1.

Fixes paxos crashes in handle_begin

mon/Paxos.cc: In function 'void Paxos::handle_begin(MMonPaxos*)', in thread '0x7fc74d11f700'
mon/Paxos.cc: 393: FAILED assert(begin->last_committed == last_committed)

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

diff --git a/src/mon/MonitorStore.cc b/src/mon/MonitorStore.cc
index b8b7e4b7a7cae97716d10ccff04d0ccd77d84eb9..19f1165bae5041b12d9c912bbc0bebc265c3ffc7 100644 (file)
--- a/src/mon/MonitorStore.cc
+++ b/src/mon/MonitorStore.cc
@@ -340,18 +340,20 @@ int MonitorStore::write_bl_ss(bufferlist& bl, const char *a, const char *b, bool
   return 0;
 }
 
-int MonitorStore::put_bl_sn_map(const char *a, map<version_t,bufferlist>& vals)
+int MonitorStore::put_bl_sn_map(const char *a,
+                               map<version_t,bufferlist>::iterator start,
+                               map<version_t,bufferlist>::iterator end)
 {
-  version_t first = vals.begin()->first;
-  version_t last = vals.rbegin()->first;
+  version_t first = start->first;
+  map<version_t,bufferlist>::iterator lastp = end;
+  --lastp;
+  version_t last = lastp->first;
   dout(15) <<  "put_bl_sn_map " << a << "/[" << first << ".." << last << "]" << dendl;
 
   // only do a big sync if there are several values.
-  if (vals.size() < 5) {
+  if (last - first < 5) {
     // just do them individually
-    for (map<version_t,bufferlist>::iterator p = vals.begin();
-        p != vals.end();
-        p++) {
+    for (map<version_t,bufferlist>::iterator p = start; p != end; ++p) {
       int err = put_bl_sn(p->second, a, p->first);
       if (err < 0)
        return err;
@@ -364,9 +366,7 @@ int MonitorStore::put_bl_sn_map(const char *a, map<version_t,bufferlist>& vals)
   snprintf(dfn, sizeof(dfn), "%s/%s", dir.c_str(), a);
   ::mkdir(dfn, 0755);
 
-  for (map<version_t,bufferlist>::iterator p = vals.begin();
-       p != vals.end();
-       p++) {
+  for (map<version_t,bufferlist>::iterator p = start; p != end; ++p) {
     char tfn[1024], fn[1024];
 
     snprintf(fn, sizeof(fn), "%s/%llu", dfn, (long long unsigned)p->first);
@@ -391,9 +391,7 @@ int MonitorStore::put_bl_sn_map(const char *a, map<version_t,bufferlist>& vals)
   ::close(dirfd);
     
   // rename them all into place
-  for (map<version_t,bufferlist>::iterator p = vals.begin();
-       p != vals.end();
-       p++) {
+  for (map<version_t,bufferlist>::iterator p = start; p != end; ++p) {
     char tfn[1024], fn[1024];
     
     snprintf(fn, sizeof(fn), "%s/%llu", dfn, (long long unsigned)p->first);

diff --git a/src/mon/MonitorStore.h b/src/mon/MonitorStore.h
index 7c19814fa6542e9094ed06a7d7cf6cf540f0f9ea..a520e587576062f9ad583f3bb7a1eb9e2af2f917 100644 (file)
--- a/src/mon/MonitorStore.h
+++ b/src/mon/MonitorStore.h
@@ -75,7 +75,9 @@ public:
    * @param vals - map of int name -> values
    * @return 0 for success or negative error code
    */
-  int put_bl_sn_map(const char *a, map<version_t,bufferlist>& vals);
+  int put_bl_sn_map(const char *a,
+                   map<version_t,bufferlist>::iterator start,
+                   map<version_t,bufferlist>::iterator end);
 
   int erase_ss(const char *a, const char *b);
   int erase_sn(const char *a, version_t b) {

diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc
index 869fa1160b0a97ce3ec107bdaf1038d72314138e..68a82fd40f3643904366ff9a685cc8be5ec846ca 100644 (file)
--- a/src/mon/Paxos.cc
+++ b/src/mon/Paxos.cc
@@ -201,31 +201,40 @@ void Paxos::store_state(MMonPaxos *m)
     last_committed = m->latest_version;
     first_committed = last_committed;
     mon->store->put_int(first_committed, machine_name, "first_committed");
+    mon->store->put_int(last_committed, machine_name, "last_committed");
+    return;
   }
 
   // build map of values to store
-  map<version_t,bufferlist> vals;
-  vals.swap(m->values);
-  if (vals.size()) {
-    if (vals.begin()->first > last_committed + 1) {
-      // drop everything if it starts in the future.
-      dout(20) << " dropping full set, it starts at " << vals.begin()->first << " > last_committed+1" << dendl;
-      vals.clear();
-    } else {
-      // drop anything we've already committed
-      if (vals.begin()->first <= last_committed) {
-       dout(20) << " dropping leading elements from " << vals.begin()->first << " to last_committed" << dendl;
-       while (!vals.empty() && vals.begin()->first <= last_committed) {
-         vals.erase(vals.begin());
-       }
-      }
-      if (!vals.empty()) {
-       mon->store->put_bl_sn_map(machine_name, vals);
-       last_committed = vals.rbegin()->first;
-      }
-    }
+  // we want to write the range [last_committed, m->last_committed] only.
+  map<version_t,bufferlist>::iterator start = m->values.begin();
+
+  if (start != m->values.end() &&
+      start->first > last_committed + 1) {
+    // ignore everything if values start in the future.
+    dout(10) << "store_state ignoring all values, they start at " << start->first
+            << " > last_committed+1" << dendl;
+    start = m->values.end();
+  }
+
+  while (start != m->values.end() &&
+        start->first <= last_committed)
+    ++start;
+
+  map<version_t,bufferlist>::iterator end = start;
+  while (end != m->values.end() &&
+        end->first <= m->last_committed) {
+    last_committed = end->first;
+    ++end;
+  }
+
+  if (start == end) {
+    dout(10) << "store_state nothing to commit" << dendl;
+  } else {
+    dout(10) << "store_state [" << start->first << ".." << last_committed << "]" << dendl;
+    mon->store->put_bl_sn_map(machine_name, start, end);
+    mon->store->put_int(last_committed, machine_name, "last_committed");
   }
-  mon->store->put_int(last_committed, machine_name, "last_committed");
 }