mon/MDSMonitor: remove redundant state change check

There are two sets of checks to state change in prepare_beacon.
Since the last commit, many of these checks are covered by
`MDSMap::state_transition_valid`. So merging these checks.

This fixes the bug that standby-replay is evicted unexpectedly.
This bug is introduced in
794d13c9ff4 (mon/MDSMonitor: reject illegal want_states from MDS)
but only reveal itself after
20509bb6c82 (MDSMonitor: handle damaged from standby-replay)

Fixes: https://tracker.ceph.com/issues/53811
Signed-off-by: 胡玮文 <huww98@outlook.com>

diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index 2e97f8ff21fdc84fec41455025f770f77d314fe3..638612df8839967a2a0585b436b56c56ea8c929c 100644 (file)
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -700,27 +700,27 @@ bool MDSMonitor::prepare_beacon(MonOpRequestRef op)
       return true;
     }
 
-    // legal state change?
-    if ((info.state == MDSMap::STATE_STANDBY && state > 0) ||
-        (info.state == MDSMap::STATE_STANDBY_REPLAY && state > 0 && state != MDSMap::STATE_DAMAGED)) {
-      /* N.B.: standby-replay can indicate the rank is damaged due to failure to replay */
-      dout(10) << "mds_beacon mds can't activate itself (" << ceph_mds_state_name(info.state)
-              << " -> " << ceph_mds_state_name(state) << ")" << dendl;
+    if (state == MDSMap::STATE_DNE) {
+      dout(1) << __func__ << ": DNE from " << info << dendl;
       goto evict;
-    } else if (info.state >= CEPH_MDS_STATE_REPLAY &&
-              (state == MDSMap::STATE_STANDBY ||
-               state == MDSMap::STATE_STANDBY_REPLAY)) {
-      dout(1) << "mds_beacon MDS can't go back into standby after taking rank: "
-                 "held rank " << info.rank << " while requesting state "
-              << ceph_mds_state_name(state) << " from "
-              << ceph_mds_state_name(info.state) << dendl;
+    }
+
+    // legal state change?
+    if ((info.state == MDSMap::STATE_STANDBY && state != info.state) ||
+        (info.state == MDSMap::STATE_STANDBY_REPLAY && state != info.state && state != MDSMap::STATE_DAMAGED)) {
+      // Standby daemons should never modify their own state.
+      // Except that standby-replay can indicate the rank is damaged due to failure to replay.
+      // Reject any attempts to do so.
+      derr << "standby " << gid << " attempted to change state to "
+           << ceph_mds_state_name(state) << ", rejecting" << dendl;
       goto evict;
-    } else if (info.state == MDSMap::STATE_STOPPING &&
-        state != MDSMap::STATE_STOPPING &&
-        state != MDSMap::STATE_STOPPED) {
-      // we can't transition to any other states from STOPPING
-      dout(0) << "got beacon for MDS in STATE_STOPPING, ignoring requested state change"
-              << dendl;
+    } else if (info.state != MDSMap::STATE_STANDBY && state != info.state &&
+               !MDSMap::state_transition_valid(info.state, state)) {
+      // Validate state transitions for daemons that hold a rank
+      derr << "daemon " << gid << " (rank " << info.rank << ") "
+           << "reported invalid state transition "
+           << ceph_mds_state_name(info.state) << " -> "
+           << ceph_mds_state_name(state) << dendl;
       goto evict;
     }
 
@@ -801,23 +801,6 @@ bool MDSMonitor::prepare_beacon(MonOpRequestRef op)
       last_beacon.erase(rankgid);
 
       /* MDS expects beacon reply back */
-    } else if (state == MDSMap::STATE_DNE) {
-      dout(1) << __func__ << ": DNE from " << info << dendl;
-      goto evict;
-    } else if (info.state == MDSMap::STATE_STANDBY && state != info.state) {
-      // Standby daemons should never modify their own
-      // state.  Reject any attempts to do so.
-      derr << "standby " << gid << " attempted to change state to "
-           << ceph_mds_state_name(state) << ", rejecting" << dendl;
-      goto evict;
-    } else if (info.state != MDSMap::STATE_STANDBY && state != info.state &&
-               !MDSMap::state_transition_valid(info.state, state)) {
-      // Validate state transitions for daemons that hold a rank
-      derr << "daemon " << gid << " (rank " << info.rank << ") "
-           << "reported invalid state transition "
-           << ceph_mds_state_name(info.state) << " -> "
-           << ceph_mds_state_name(state) << dendl;
-      goto evict;
     } else {
       if (info.state != MDSMap::STATE_ACTIVE && state == MDSMap::STATE_ACTIVE) {
         const auto &fscid = pending.mds_roles.at(gid);