mon: MonmapMonitor: don't rerun command if we updated & bootstrappedwip-5896

MonMap changes lead to bootstraps.  Callbacks waiting for a proposal to
finish can have several fates, depending on what happens: finished, rerun
or aborted.

In the case of a bootstrap right after a monmap change, callbacks are
rerun.  Considering we queued the message that lead to the monmap change
on this queue, if we instead of finishing it end up reruning it, we will
end up trying to perform the same modification twice -- the last one will
try to modify an already existing state and we will return just that:
whatever you're attempting to do has already been done.

This patch adds a new callback class that will check if we are attempting
at rerunning a monmap change, and if so, we will check if what we're
attempting to do has already been accomplished by the message we're
rerunning;  if so, we finish the message instead.

Fixes: #5896

Signed-off-by: Joao Eduardo Luis <jecluis@gmail.com>

2 files changed, 44 insertions, 3 deletions

diff --git a/src/mon/MonmapMonitor.cc b/src/mon/MonmapMonitor.cc
index 799f19df154..26fb0dcbb6a 100644
--- a/src/mon/MonmapMonitor.cc
+++ b/src/mon/MonmapMonitor.cc
@@ -298,12 +298,27 @@ bool MonmapMonitor::prepare_command(MMonCommand *m)
       addr.set_port(CEPH_MON_PORT);
     }
 
+    do {
+      if (pending_map.contains(addr)) {
+        string n = pending_map.get_name(addr);
+        if (n == name)
+          break;
+      } else if (pending_map.contains(name))
+    } while (false);
+
     if (pending_map.contains(addr) ||
 	pending_map.contains(name)) {
-      err = -EEXIST;
       if (!ss.str().empty())
 	ss << "; ";
-      ss << "mon " << name << " " << addr << " already exists";
+
+      string n = pending_map.get_name(addr);
+      if (n == name) {
+        err = 0;
+        ss << "added mon." << name << " at " << addr;
+      } else {
+        err = -EEXIST;
+        ss << "mon." << name << " at " << addr << " already exists";
+      }
       goto out;
     }
 
@@ -311,7 +326,9 @@ bool MonmapMonitor::prepare_command(MMonCommand *m)
     pending_map.last_changed = ceph_clock_now(g_ceph_context);
     ss << "added mon." << name << " at " << addr;
     getline(ss, rs);
-    wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs, get_last_committed()));
+    wait_for_finished_proposal(new C_MonmapCommand(mon, m, 0, rs,
+                                                   get_last_committed(),
+                                                   pending_map.last_changed));
     return true;
 
   } else if (prefix == "mon remove") {
diff --git a/src/mon/MonmapMonitor.h b/src/mon/MonmapMonitor.h
index 198489d7017..a0fccfd56f2 100644
--- a/src/mon/MonmapMonitor.h
+++ b/src/mon/MonmapMonitor.h
@@ -30,6 +30,7 @@ using namespace std;
 #include "PaxosService.h"
 #include "MonMap.h"
 #include "MonitorDBStore.h"
+#include "Monitor.h"
 
 class MMonGetMap;
 class MMonMap;
@@ -81,6 +82,29 @@ class MonmapMonitor : public PaxosService {
 
   void tick();
 
+  struct C_MonmapCommand : public Monitor::C_Command {
+    utime_t last_changed;
+
+    C_MonmapCommand(Monitor *_mon, MMonCommand *_m, int r,
+                    string s, version_t v, utime_t changed) :
+      Monitor::C_Command(_mon, _m, r, s, v),
+      last_changed(changed)
+    { }
+
+    void finish(int r) {
+      int ret = r;
+      if (r == -EAGAIN) {
+        MonMap m;
+        mon->monmon()->get_monmap(m);
+        if ((m.get_epoch() == version+1)
+            && (m.last_changed == last_changed)) {
+          ret = 0;
+        }
+      }
+      Monitor::C_Command::finish(ret);
+    }
+  };
+
  private:
   bufferlist monmap_bl;
 };