mon: MDSMonitor: check all conditions are met *before* osdmon proposal

We should not allow ourselves to request the osdmon to propose before we
know for sure that we meet the required conditions to go through with
our own state change.  Even if we still can't guarantee that our
proposal is going to be committed, we shouldn't just change the osdmon's
state just because we can.  This way, at least, we make sure that our
checks hold up before doing anything with side-effects.

Fixes: #10643
Signed-off-by: Joao Eduardo Luis <joao@redhat.com>
(cherry picked from commit 1d1ae413e9f0aafa314af903bcf6b815f6558082)

diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index c0b831f1043c02e6dda536724d45a91b4ecea579..c488a8928c07a8dceb630ea80e89984d48a0d6f2 100644 (file)
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -1116,14 +1116,11 @@ bool MDSMonitor::management_command(
     pg_pool_t const *metadata_pool = mon->osdmon()->osdmap.get_pg_pool(metadata);
     assert(metadata_pool != NULL);  // Checked it existed above
 
-    // Automatically set crash_replay_interval on data pool if it
-    // isn't already set.
-    if (data_pool->get_crash_replay_interval() == 0) {
-      r = mon->osdmon()->set_crash_replay_interval(data, g_conf->osd_default_data_pool_replay_window);
-      assert(r == 0);  // We just did get_pg_pool so it must exist and be settable
-      request_proposal(mon->osdmon());
-    }
-
+    // we must make these checks before we even allow ourselves to *think*
+    // about requesting a proposal to the osdmonitor and bail out now if
+    // we believe we must.  bailing out *after* we request the proposal is
+    // bad business as we could have changed the osdmon's state and ending up
+    // returning an error to the user.
     r = _check_pool(data, &ss);
     if (r) {
       return true;
@@ -1134,6 +1131,14 @@ bool MDSMonitor::management_command(
       return true;
     }
 
+    // Automatically set crash_replay_interval on data pool if it
+    // isn't already set.
+    if (data_pool->get_crash_replay_interval() == 0) {
+      r = mon->osdmon()->set_crash_replay_interval(data, g_conf->osd_default_data_pool_replay_window);
+      assert(r == 0);  // We just did get_pg_pool so it must exist and be settable
+      request_proposal(mon->osdmon());
+    }
+
     // All checks passed, go ahead and create.
     MDSMap newmap;
     newmap.inc = pending_mdsmap.inc;