From: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
Date: Tue, 2 Jun 2026 11:43:14 +0000 (+0000)
Subject: mon: fix ConnectionTracker::notify_rank_removed
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c72f9a86b8833e3b2d159e1411ef68fbfbe8ed5c;p=ceph.git

mon: fix ConnectionTracker::notify_rank_removed

Problem:
When multiple monitors are removed from the monmap in a single update,
the iterative rank adjustment in notify_rank_removed() fails to maintain
the invariant that our local rank matches the monmap's new_rank until
all iterations complete.

Problem:
The old code decremented rank for each removed rank:
```
  for (auto i = monmap->removed_ranks.rbegin(); ...) {
    int remove_rank = *i;
    int new_rank = monmap->get_rank(messenger->get_myaddrs());

    // Old logic (in ConnectionTracker):
    if (remove_rank < rank) {
      --rank;
    }

    // This assert would fail on first iteration when removed_ranks.size() > 1
    ceph_assert(rank == new_rank);
  }
```

When removed_ranks = {0, 1} and we are mon.d (originally rank 3):
- new_rank from monmap = 1 (final correct value after removing 0 and 1)

Iteration 1 (remove rank 1):
  Before: rank = 3
  Execute: rank_removed=1 < 3, so --rank -> rank = 2
  Check: rank == new_rank? -> 2 == 1? FAIL

Iteration 2 (remove rank 0):
  Before: rank = 2
  Execute: rank_removed=0 < 2, so --rank -> rank = 1
  Check: rank == new_rank? → 1 == 1? PASS

The invariant is violated during intermediate iterations because each
--rank adjustment only accounts for ONE removed rank, but new_rank
from monmap already accounts for ALL removed ranks.

Solution:
Trust the caller's new_rank from monmap instead of trying to compute it
iteratively. The monmap has already recalculated ranks correctly by
removing all monitors atomically, so we should just use that value.

Changed:
```
  if (rank_removed < rank) {
    --rank;
  }
```
To:
  // Trust the caller's new_rank from the monmap rather than trying to compute it.
  // This handles cases where multiple ranks are removed simultaneously.
  rank = new_rank;

Now the invariant holds on every iteration regardless of how many ranks
are removed.

Fixes: https://tracker.ceph.com/issues/77423

Signed-off-by: Kamoltat (Junior) Sirivadhna <ksirivad@redhat.com>
---

diff --git a/src/mon/ConnectionTracker.cc b/src/mon/ConnectionTracker.cc
index e78b5be3086..9cc05054357 100644
--- a/src/mon/ConnectionTracker.cc
+++ b/src/mon/ConnectionTracker.cc
@@ -261,18 +261,15 @@ void ConnectionTracker::notify_rank_removed(int rank_removed, int new_rank)
   ceph_assert((peer_reports.size() == starting_size) ||
 	  (peer_reports.size() + 1 == starting_size));
 
-  if (rank_removed < rank) { // if the rank removed is lower than us, we need to adjust.
-    --rank;
-    my_reports.rank = rank; // also adjust my_reports.rank.
-  }
+  // Trust the caller's new_rank from the monmap rather than trying to compute it.
+  // This handles cases where multiple ranks are removed simultaneously.
+  rank = new_rank;
+  my_reports.rank = new_rank;
 
   ldout(cct, 20) << "my rank after: " << rank << dendl;
   ldout(cct, 20) << "peer_reports after: " << peer_reports << dendl;
   ldout(cct, 20) << "my_reports after: " << my_reports << dendl;
 
-  //check if the new_rank from monmap is equal to our adjusted rank.
-  ceph_assert(rank == new_rank);
-
   increase_version();
 }