From 5d17185522a519d678ac2d0e1bb539908b1594ec Mon Sep 17 00:00:00 2001 From: Kamoltat Date: Thu, 13 Jun 2024 16:17:53 +0000 Subject: [PATCH] mon/ElectionLogic: tie-breaker ignore proposal from marked down mon Problem: Monitor election gets stuck, resulting in the cluster being unaccessible. Set up stretch cluster: DC1: mon.a,mon.b DC2: mon.c, mon.d Arbiter: mon.e Apply netsplit between DC1 and DC2, wait around 10 seconds, client tried accessing the cluster ... failed to access. We expect the cluster to be functional when there is netsplit. This was due to how we suppose to kick one DC out of quorum and keep them in the dark until the connection comes back (netsplit gone). However, there is a small window where the out-of-quorum DC is able to have an influence on the tiebreaker MON by forcing the tiebreaker MON to bump its epoch which then it will reject the winner MON's victory message from in-quorum DC. Solution: In CONNECTIVITY election strategy, tie-breaker MON disregard any proposal that is coming from MONs that belongs to `stretch_marked_down_mons` set. As a result, the out-of-quorum MONs won't get the chance to force the tie-breaker MON to increase its epoch, hence will now accept the victory message from the in-quorum Monitors. Also, edited src/test/mon/test_election.cc to include `is_stretch_marked_down_mon` and `is_tiebreaker`, preventing the make check from breaking. Fixes: https://tracker.ceph.com/issues/66471 Signed-off-by: Kamoltat (cherry picked from commit d3608533672d575fbc54fc965ddde93dfdb89721) --- src/mon/ElectionLogic.cc | 6 ++++++ src/mon/ElectionLogic.h | 12 ++++++++++++ src/mon/Elector.cc | 16 ++++++++++++++++ src/mon/Elector.h | 13 +++++++++++++ src/mon/MonMap.h | 3 ++- src/test/mon/test_election.cc | 14 ++++++++++++++ 6 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/mon/ElectionLogic.cc b/src/mon/ElectionLogic.cc index 0c1b30c417c5e..9f174de2d9307 100644 --- a/src/mon/ElectionLogic.cc +++ b/src/mon/ElectionLogic.cc @@ -335,6 +335,12 @@ void ElectionLogic::propose_connectivity_handler(int from, epoch_t mepoch, ldout(cct, 10) << __func__ << " from " << from << " mepoch: " << mepoch << " epoch: " << epoch << dendl; ldout(cct, 30) << "last_election_winner: " << last_election_winner << dendl; + // ignore proposal from marked down mons if we are the tiebreaker + if (elector->is_tiebreaker(elector->get_my_rank()) && + elector->is_stretch_marked_down_mons(from)) { + ldout(cct, 10) << "Ignoring proposal from marked down mon " << from << dendl; + return; + } if ((epoch % 2 == 0) && last_election_winner != elector->get_my_rank() && !elector->is_current_member(from)) { diff --git a/src/mon/ElectionLogic.h b/src/mon/ElectionLogic.h index e2f2db82ac88f..af762d4fbc271 100644 --- a/src/mon/ElectionLogic.h +++ b/src/mon/ElectionLogic.h @@ -82,6 +82,18 @@ public: * @returns true if we have participated, false otherwise */ virtual bool ever_participated() const = 0; + /** + * Check if the monitor is the tiebreaker in a stretch cluster. + * + * @returns true if the Monitor is the tiebreaker, false otherwise. + */ + virtual bool is_tiebreaker(int rank) const = 0; + /** + * Check if the Monitor is marked down in a stretch cluster. + * + * @returns true if the Monitor in a stretch cluster is marked down, false otherwise. + */ + virtual bool is_stretch_marked_down_mons(int rank) const = 0; /** * Ask the ElectionOwner for the size of the Paxos set. This includes * those monitors which may not be in the current quorum! diff --git a/src/mon/Elector.cc b/src/mon/Elector.cc index 5ad30ff225c82..bba7118cb3fb0 100644 --- a/src/mon/Elector.cc +++ b/src/mon/Elector.cc @@ -722,6 +722,22 @@ bool Elector::peer_tracker_is_clean() return peer_tracker.is_clean(mon->rank, paxos_size()); } +bool Elector::is_tiebreaker(int rank) const +{ + return mon->monmap->tiebreaker_mon == mon->monmap->get_name(rank); +} + +bool Elector::is_stretch_marked_down_mons(int rank) const +{ + std::string mon_name = mon->monmap->get_name(rank); + for (auto& i : mon->monmap->stretch_marked_down_mons) { + if (i == mon_name) { + return true; + } + } + return false; +} + void Elector::notify_clear_peer_state() { dout(10) << __func__ << dendl; diff --git a/src/mon/Elector.h b/src/mon/Elector.h index be2f91c0f93dd..faeb2c58c0e2e 100644 --- a/src/mon/Elector.h +++ b/src/mon/Elector.h @@ -244,6 +244,19 @@ class Elector : public ElectionOwner, RankProvider { /* Right now we don't disallow anybody */ std::set disallowed_leaders; const std::set& get_disallowed_leaders() const { return disallowed_leaders; } + /** + * Check if the monitor is the tiebreaker in a stretch cluster. + * + * @returns true if the Monitor is the tiebreaker, false otherwise. + */ + bool is_tiebreaker(int rank) const; + /** + * Check if the mon is marked dwon in stretch mode. + * + * @returns true if the monitor is marked down in stretch mode, + * otherwise return false. + */ + bool is_stretch_marked_down_mons(int from) const; /** * Reset the expire_event timer so we can limit the amount of time we * will be electing. Clean up our peer_info. diff --git a/src/mon/MonMap.h b/src/mon/MonMap.h index 5bd72b1d917f2..34f160c1ffd74 100644 --- a/src/mon/MonMap.h +++ b/src/mon/MonMap.h @@ -165,7 +165,8 @@ class MonMap { std::set disallowed_leaders; // can't be leader under CONNECTIVITY/DISALLOW bool stretch_mode_enabled = false; std::string tiebreaker_mon; - std::set stretch_marked_down_mons; // can't be leader until fully recovered + std::set stretch_marked_down_mons; // can't be leader or taken proposal in CONNECTIVITY + // seriously until fully recovered public: void calc_legacy_ranks(); diff --git a/src/test/mon/test_election.cc b/src/test/mon/test_election.cc index 9dba99136e358..c89ed79559ddf 100644 --- a/src/test/mon/test_election.cc +++ b/src/test/mon/test_election.cc @@ -103,6 +103,8 @@ struct Owner : public ElectionOwner, RankProvider { bool timer_election; // the timeout is for normal election, or victory bool rank_deleted = false; string prefix_str; + set stretch_marked_down_mons; + int tiebreaker_mon_rank; Owner(int r, ElectionLogic::election_strategy es, double tracker_halflife, Election *p) : parent(p), rank(r), persisted_epoch(0), ever_joined(false), @@ -187,6 +189,18 @@ struct Owner : public ElectionOwner, RankProvider { quorum = members; victory_accepters = 1; } + bool is_stretch_marked_down_mons(int rank) const { + for (auto& i : stretch_marked_down_mons) { + if (i == rank) { + return true; + } + } + return false; + } + bool is_tiebreaker(int rank) const + { + return tiebreaker_mon_rank == rank; + } bool is_current_member(int r) const { return quorum.count(r) != 0; } void receive_propose(int from, epoch_t e, ConnectionTracker *oct) { if (rank_deleted) return; -- 2.39.5