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 <ksirivad@redhat.com>

diff --git a/src/mon/ElectionLogic.cc b/src/mon/ElectionLogic.cc
index 0c1b30c417c5eb462e37d9975d9d061727aec604..9f174de2d93077d3f405bbe8164362cdcb780ad2 100644 (file)
--- 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 e2f2db82ac88fe54632e5977b26d65efa5e51e13..af762d4fbc27137a6484ddd48d41f1c2dad165c1 100644 (file)
--- 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 5ad30ff225c82cf8f9b1576c3465e395c805b160..bba7118cb3fb0fa16c5708932b08c92407ab57e0 100644 (file)
--- 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 be2f91c0f93dd400954503fef8a40e9d4dbb538c..faeb2c58c0e2e85c2aa5b0c3b05be3e48742d70d 100644 (file)
--- 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<int> disallowed_leaders;
   const std::set<int>& 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 5bd72b1d917f2a82083adf3ca38db17fd971d29d..34f160c1ffd746d578916b4b3743f9a1f2fe5aff 100644 (file)
--- a/src/mon/MonMap.h
+++ b/src/mon/MonMap.h
@@ -165,7 +165,8 @@ class MonMap {
   std::set<std::string> disallowed_leaders; // can't be leader under CONNECTIVITY/DISALLOW
   bool stretch_mode_enabled = false;
   std::string tiebreaker_mon;
-  std::set<std::string> stretch_marked_down_mons; // can't be leader until fully recovered
+  std::set<std::string> 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 167d0fabf11f01068c4cfd27f0b30d65da0b7b32..9cbcce09e19bfa8c1819491aadf9d084973c7ed6 100644 (file)
--- 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<int> 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;