mon: clear connection score during update & add sanity check live/dead connection...

When upgrading the monitors (include booting up),
we check if `peer_tracker` is dirty or not. If
so, we clear it. Added some functions in `Elector` and
`ConnectionTracker` class to
check for clean `peer_tracker`.

Moreover, there could be some cases where due
to startup weirdness or abnormal circumstances,
we might get a report from our own rank. Therefore,
it doesn't hurt to add a sanity check in
`ConnectionTracker::report_live_connection` and
`ConnectionTracker::report_dead_connection`.

Fixes: https://tracker.ceph.com/issues/58049
Signed-off-by: Kamoltat <ksirivad@redhat.com>

diff --git a/src/mon/ConnectionTracker.cc b/src/mon/ConnectionTracker.cc
index 022522aa9701469fa551781b239abde3cba89cc0..272ad40c27461498d45dc7705cbb29231c7dc47e 100644 (file)
--- a/src/mon/ConnectionTracker.cc
+++ b/src/mon/ConnectionTracker.cc
@@ -106,6 +106,10 @@ void ConnectionTracker::report_live_connection(int peer_rank, double units_alive
 {
   ldout(cct, 30) << __func__ << " peer_rank: " << peer_rank << " units_alive: " << units_alive << dendl;
   ldout(cct, 30) << "my_reports before: " << my_reports << dendl;
+  if (peer_rank == rank) {
+    lderr(cct) << "Got a report from my own rank, hopefully this is startup weirdness, dropping" << dendl;
+    return;
+  }
   // we need to "auto-initialize" to 1, do shenanigans
   auto i = my_reports.history.find(peer_rank);
   if (i == my_reports.history.end()) {
@@ -130,6 +134,10 @@ void ConnectionTracker::report_dead_connection(int peer_rank, double units_dead)
 {
   ldout(cct, 30) << __func__ << " peer_rank: " << peer_rank << " units_dead: " << units_dead << dendl;
   ldout(cct, 30) << "my_reports before: " << my_reports << dendl;
+  if (peer_rank == rank) {
+    lderr(cct) << "Got a report from my own rank, hopefully this is startup weirdness, dropping" << dendl;
+    return;
+  }
   // we need to "auto-initialize" to 1, do shenanigans
   auto i = my_reports.history.find(peer_rank);
   if (i == my_reports.history.end()) {
@@ -246,6 +254,22 @@ void ConnectionTracker::notify_rank_removed(int rank_removed, int new_rank)
   increase_version();
 }
 
+bool ConnectionTracker::is_clean(int mon_rank, int monmap_size)
+{
+  ldout(cct, 30) << __func__ << dendl;
+  // check consistency between our rank according
+  // to monmap and our rank according to our report.
+  if (rank != mon_rank ||
+    my_reports.rank != mon_rank) {
+    return false;
+  } else if (!peer_reports.empty()){
+    // if peer_report max rank is greater than monmap max rank
+    // then there is a problem.
+    if (peer_reports.rbegin()->first > monmap_size - 1) return false;
+  }
+  return true;
+}
+
 void ConnectionTracker::encode(bufferlist &bl) const
 {
   ENCODE_START(1, 1, bl);

diff --git a/src/mon/ConnectionTracker.h b/src/mon/ConnectionTracker.h
index 4196bca279341a3ee0793e02b9a2953b7b0a3b65..c1a32c08fe0617f0ed56cc24a4b53c24ac111225 100644 (file)
--- a/src/mon/ConnectionTracker.h
+++ b/src/mon/ConnectionTracker.h
@@ -120,6 +120,13 @@ class ConnectionTracker {
    */
   void get_total_connection_score(int peer_rank, double *rating,
                                  int *live_count) const;
+  /**
+  * Check if our ranks are clean and make
+  * sure there are no extra peer_report lingering.
+  * In the future we also want to check the reports
+  * current and history of each peer_report.
+  */
+  bool is_clean(int mon_rank, int monmap_size);
   /**
    * Encode this ConnectionTracker. Useful both for storing on disk
    * and for sending off to peers for decoding and import
@@ -185,6 +192,7 @@ class ConnectionTracker {
     rank = new_rank;
     my_reports.rank = rank;
   }
+
   void notify_rank_changed(int new_rank);
   void notify_rank_removed(int rank_removed, int new_rank);
   friend std::ostream& operator<<(std::ostream& o, const ConnectionTracker& c);

diff --git a/src/mon/Elector.cc b/src/mon/Elector.cc
index b5fa55a925071983bd2d72cbaa111b5c316d81db..9ef74d70a4e2392951829fa64811b8ca2972833d 100644 (file)
--- a/src/mon/Elector.cc
+++ b/src/mon/Elector.cc
@@ -716,6 +716,11 @@ void Elector::start_participating()
   logic.participating = true;
 }
 
+bool Elector::peer_tracker_is_clean()
+{
+  return peer_tracker.is_clean(mon->rank, paxos_size());
+}
+
 void Elector::notify_clear_peer_state()
 {
   dout(10) << __func__ << dendl;

diff --git a/src/mon/Elector.h b/src/mon/Elector.h
index 60afa5dd60c5516cb099c7f72fb80ffaa04a8784..be5eee5d4dadaedb48332f362765b1576b265a31 100644 (file)
--- a/src/mon/Elector.h
+++ b/src/mon/Elector.h
@@ -357,6 +357,11 @@ class Elector : public ElectionOwner, RankProvider {
    * @post  @p participating is true
    */
   void start_participating();
+  /**
+  * Check if our peer_tracker is self-consistent, not suffering from
+  * https://tracker.ceph.com/issues/58049
+  */
+  bool peer_tracker_is_clean();
   /**
    * Forget everything about our peers. :(
    */

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index e12b7f0102c79c3482c10af4a5bac39678066c7c..0a2a9d336ef196422c6f2408ab571a052b7b0cce 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -943,7 +943,15 @@ int Monitor::init()
   osdmon()->get_filestore_osd_list();
 
   state = STATE_PROBING;
+
   bootstrap();
+
+  if (!elector.peer_tracker_is_clean()){
+    dout(10) << "peer_tracker looks inconsistent"
+      << " previous bad logic, clearing ..." << dendl;
+    elector.notify_clear_peer_state();
+  }
+
   // add features of myself into feature_map
   session_map.feature_map.add_mon(con_self->get_features());
   return 0;