From 6359c566357fff1e4f7209b6d48fa44be46701cb Mon Sep 17 00:00:00 2001
From: Patrick Donnelly <pdonnell@redhat.com>
Date: Wed, 5 Apr 2023 16:30:48 -0400
Subject: [PATCH] mon/MDSMonitor: plug paxos when maybe manipulating the osdmap

Instead of tracking where exactly we're plugging PAXOS, just do it
unconditionally whenever we modify pending and may change the osdmap.
There is no downside to doing so; it simplifies the code.

Fixes: https://tracker.ceph.com/issues/59314
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
(cherry picked from commit 885005ba647019053016b1de196d83e23b9efb2b)
---
 src/mon/FSCommands.cc | 12 ---------
 src/mon/FSCommands.h  |  4 ---
 src/mon/MDSMonitor.cc | 60 ++++++++++++++++++++++---------------------
 3 files changed, 31 insertions(+), 45 deletions(-)

diff --git a/src/mon/FSCommands.cc b/src/mon/FSCommands.cc
index 5acafb1ff91d..675bad6a9455 100644
--- a/src/mon/FSCommands.cc
+++ b/src/mon/FSCommands.cc
@@ -146,10 +146,6 @@ class FsNewHandler : public FileSystemCommandHandler
   {
   }
 
-  bool batched_propose() override {
-    return true;
-  }
-
   int handle(
       Monitor *mon,
       FSMap& fsmap,
@@ -871,10 +867,6 @@ class AddDataPoolHandler : public FileSystemCommandHandler
     : FileSystemCommandHandler("fs add_data_pool"), m_paxos(paxos)
   {}
 
-  bool batched_propose() override {
-    return true;
-  }
-
   int handle(
       Monitor *mon,
       FSMap& fsmap,
@@ -1096,10 +1088,6 @@ class RenameFilesystemHandler : public FileSystemCommandHandler
   {
   }
 
-  bool batched_propose() override {
-    return true;
-  }
-
   int handle(
       Monitor *mon,
       FSMap& fsmap,
diff --git a/src/mon/FSCommands.h b/src/mon/FSCommands.h
index 4b59225f9545..44dff4e4cdd2 100644
--- a/src/mon/FSCommands.h
+++ b/src/mon/FSCommands.h
@@ -78,10 +78,6 @@ public:
 
   static std::list<std::shared_ptr<FileSystemCommandHandler> > load(Paxos *paxos);
 
-  virtual bool batched_propose() {
-    return false;
-  }
-
   virtual int handle(
     Monitor *mon,
     FSMap &fsmap,
diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index 385f831710b4..25339b025710 100644
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -550,28 +550,35 @@ bool MDSMonitor::prepare_update(MonOpRequestRef op)
   auto m = op->get_req<PaxosServiceMessage>();
   dout(7) << "prepare_update " << *m << dendl;
 
-  switch (m->get_type()) {
-    
-  case MSG_MDS_BEACON:
-    return prepare_beacon(op);
+  bool r = false;
 
-  case MSG_MON_COMMAND:
-    try {
-      return prepare_command(op);
-    } catch (const bad_cmd_get& e) {
-      bufferlist bl;
-      mon.reply_command(op, -EINVAL, e.what(), bl, get_last_committed());
-      return false; /* nothing to propose */
-    }
+  /* batch any changes to pending with any changes to osdmap */
+  paxos.plug();
 
-  case MSG_MDS_OFFLOAD_TARGETS:
-    return prepare_offload_targets(op);
-  
-  default:
-    ceph_abort();
+  switch (m->get_type()) {
+    case MSG_MDS_BEACON:
+      r = prepare_beacon(op);
+      break;
+    case MSG_MON_COMMAND:
+      try {
+        r = prepare_command(op);
+      } catch (const bad_cmd_get& e) {
+        bufferlist bl;
+        mon.reply_command(op, -EINVAL, e.what(), bl, get_last_committed());
+        r = false;
+      }
+      break;
+    case MSG_MDS_OFFLOAD_TARGETS:
+      r = prepare_offload_targets(op);
+      break;
+    default:
+      ceph_abort();
+      break;
   }
 
-  return false; /* nothing to propose! */
+  paxos.unplug();
+
+  return r;
 }
 
 bool MDSMonitor::prepare_beacon(MonOpRequestRef op)
@@ -1389,7 +1396,6 @@ bool MDSMonitor::prepare_command(MonOpRequestRef op)
 
   auto &pending = get_pending_fsmap_writeable();
 
-  bool batched_propose = false;
   for (const auto &h : handlers) {
     r = h->can_handle(prefix, op, pending, cmdmap, ss);
     if (r == 1) {
@@ -1400,14 +1406,7 @@ bool MDSMonitor::prepare_command(MonOpRequestRef op)
       goto out;
     }
 
-    batched_propose = h->batched_propose();
-    if (batched_propose) {
-      paxos.plug();
-    }
     r = h->handle(&mon, pending, op, cmdmap, ss);
-    if (batched_propose) {
-      paxos.unplug();
-    }
 
     if (r == -EAGAIN) {
       // message has been enqueued for retry; return.
@@ -1448,9 +1447,6 @@ out:
     // success.. delay reply
     wait_for_finished_proposal(op, new Monitor::C_Command(mon, op, r, rs,
 					      get_last_committed() + 1));
-    if (batched_propose) {
-      force_immediate_propose();
-    }
     return true;
   } else {
     // reply immediately
@@ -2321,6 +2317,9 @@ void MDSMonitor::tick()
 
   auto &pending = get_pending_fsmap_writeable();
 
+  /* batch any changes to pending with any changes to osdmap */
+  paxos.plug();
+
   bool do_propose = false;
   bool propose_osdmap = false;
 
@@ -2376,6 +2375,9 @@ void MDSMonitor::tick()
     request_proposal(mon.osdmon());
   }
 
+  /* allow MDSMonitor::propose_pending() to push the proposal through */
+  paxos.unplug();
+
   if (do_propose) {
     propose_pending();
   }
-- 
2.47.3