From bf75a9ea08084afe4a02083473a7146cb91dae3b Mon Sep 17 00:00:00 2001 From: =?utf8?q?=E8=83=A1=E7=8E=AE=E6=96=87?= Date: Sun, 9 Jan 2022 23:17:38 +0800 Subject: [PATCH] mon/MDSMonitor: remove redundant state change check MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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: 胡玮文 --- src/mon/MDSMonitor.cc | 55 +++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc index 2e97f8ff21f..638612df883 100644 --- 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); -- 2.39.5