osd: Fix race hazard that caused duplicated message after OSD marked down

After an OSD that has not crashed is marked down there is a race hazard
where a message can be deliever to the OSD a second time. Different
asserts are hit depending on which message is duplicated.

This issue is seen in tests which mark osds down as an error inject
and very occasionally in the field where there are other issues
(e.g. network problems) that are causing osds which have not
crashed to be marked down.

The fix is to change the order in which the OSD processes a
new epoch and OSDMap which marks the OSD as down, it must
first consume the map so that old messages will be filtered
out and then call rebind to reset all the connections.

Fixes: https://tracker.ceph.com/issues/58417
Assisted-by: IBM Bob 2.0.1
Signed-off-by: Bill Scales <bill_scales@uk.ibm.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index e6e7489a6753db68b156337f458f807be10ea4f7..a01afd9986c61a02691da92c7364d552406447ad 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -8637,6 +8637,7 @@ void OSD::_committed_osd_maps(epoch_t first, epoch_t last, MOSDMap *m)
   bool do_shutdown = false;
   bool do_restart = false;
   bool network_error = false;
+  bool need_rebind = false;
   OSDMapRef osdmap = get_osdmap();
 
   // advance through the new maps
@@ -8809,22 +8810,7 @@ void OSD::_committed_osd_maps(epoch_t first, epoch_t last, MOSDMap *m)
 
        start_waiting_for_healthy();
 
-       set<int> avoid_ports;
-#if defined(__FreeBSD__)
-        // prevent FreeBSD from grabbing the client_messenger port during
-        // rebinding. In which case a cluster_meesneger will connect also
-       // to the same port
-       client_messenger->get_myaddrs().get_ports(&avoid_ports);
-#endif
-       cluster_messenger->get_myaddrs().get_ports(&avoid_ports);
-
-       int r = cluster_messenger->rebind(avoid_ports);
-       if (r != 0) {
-         do_shutdown = true;  // FIXME: do_restart?
-          network_error = true;
-          derr << __func__ << " marked down:"
-              << " rebind cluster_messenger failed" << dendl;
-        }
+       need_rebind = true;
 
        hb_back_server_messenger->mark_down_all();
        hb_front_server_messenger->mark_down_all();
@@ -8846,6 +8832,28 @@ void OSD::_committed_osd_maps(epoch_t first, epoch_t last, MOSDMap *m)
   // yay!
   consume_map();
 
+  if (need_rebind) {
+    // Rebind cluster_messenger after consume_map() so that all shards
+    // have the updated OSDMap before peers can reconnect and retransmit
+    // messages. See https://tracker.ceph.com/issues/58417
+    set<int> avoid_ports;
+#if defined(__FreeBSD__)
+    // prevent FreeBSD from grabbing the client_messenger port during
+    // rebinding. In which case a cluster_messenger will connect also
+    // to the same port
+    client_messenger->get_myaddrs().get_ports(&avoid_ports);
+#endif
+    cluster_messenger->get_myaddrs().get_ports(&avoid_ports);
+
+    int r = cluster_messenger->rebind(avoid_ports);
+    if (r != 0) {
+      do_shutdown = true;  // FIXME: do_restart?
+      network_error = true;
+      derr << __func__ << " marked down:"
+          << " rebind cluster_messenger failed" << dendl;
+    }
+  }
+
   if (is_active() || is_waiting_for_healthy())
     maybe_update_heartbeat_peers();