From 5906a57320f04f57a38eef9588bd16ac3fd4e55d Mon Sep 17 00:00:00 2001
From: Neha Ojha <nojha@redhat.com>
Date: Thu, 5 Sep 2019 20:35:51 -0700
Subject: [PATCH] osd/MissingLoc.cc: do not rely on missing_loc_sources only

In 624ade487ea4aeaf988cc1767e0b293f76addd5b, we relied on missing_loc_sources
to check for strays and remove an OSD from missing_loc. However, it is
possible that missing_loc_sources is empty while there are still OSDs
present in missing_loc. Since the aim is to just remove a stray OSD from
missing_loc, we do not need to rely on missing_loc_sources. We still
clean missing_loc_sources if any stray is present in it.

Signed-off-by: Neha Ojha <nojha@redhat.com>
---
 src/osd/MissingLoc.cc | 64 +++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 36 deletions(-)

diff --git a/src/osd/MissingLoc.cc b/src/osd/MissingLoc.cc
index c5929d6be7e..8e96f0fb8be 100644
--- a/src/osd/MissingLoc.cc
+++ b/src/osd/MissingLoc.cc
@@ -187,7 +187,33 @@ void MissingLoc::check_recovery_sources(const OSDMapRef& osdmap)
 
 void MissingLoc::remove_stray_recovery_sources(pg_shard_t stray)
 {
-  set<pg_shard_t> now_stray;
+  ldout(cct, 10) << __func__ << " remove osd " << stray << " from missing_loc" << dendl;
+  // filter missing_loc
+  map<hobject_t, set<pg_shard_t>>::iterator p = missing_loc.begin();
+  while (p != missing_loc.end()) {
+    set<pg_shard_t>::iterator q = p->second.begin();
+    bool changed = false;
+    while (q != p->second.end()) {
+      if (*q == stray) {
+        if (!changed) {
+          changed = true;
+          _dec_count(p->second);
+        }
+        p->second.erase(q++);
+      } else {
+        ++q;
+      }
+    }
+    if (p->second.empty()) {
+      missing_loc.erase(p++);
+    } else {
+      if (changed) {
+        _inc_count(p->second);
+      }
+      ++p;
+    }
+  }
+  // filter missing_loc_sources
   for (set<pg_shard_t>::iterator p = missing_loc_sources.begin();
        p != missing_loc_sources.end();
        ) {
@@ -195,41 +221,7 @@ void MissingLoc::remove_stray_recovery_sources(pg_shard_t stray)
       ++p;
       continue;
     }
-    ldout(cct, 10) << __func__ << " source osd." << *p << " now stray" << dendl;
-    now_stray.insert(*p);
+    ldout(cct, 10) << __func__ << " remove osd" << stray << " from missing_loc_sources" << dendl;
     missing_loc_sources.erase(p++);
   }
-
-  if (now_stray.empty()) {
-    ldout(cct, 10) << __func__ << " no source osds (" << missing_loc_sources << ") became stray" << dendl;
-  } else {
-    ldout(cct, 10) << __func__ << " sources osds " << now_stray << " now stray, remaining sources are "
-                       << missing_loc_sources << dendl;
-
-    // filter missing_loc
-    map<hobject_t, set<pg_shard_t>>::iterator p = missing_loc.begin();
-    while (p != missing_loc.end()) {
-      set<pg_shard_t>::iterator q = p->second.begin();
-      bool changed = false;
-      while (q != p->second.end()) {
-        if (now_stray.count(*q)) {
-          if (!changed) {
-            changed = true;
-            _dec_count(p->second);
-          }
-          p->second.erase(q++);
-        } else {
-          ++q;
-        }
-      }
-      if (p->second.empty()) {
-        missing_loc.erase(p++);
-      } else {
-        if (changed) {
-          _inc_count(p->second);
-        }
-        ++p;
-      }
-    }
-  }
 }
-- 
2.39.5