PG: Handle repair once in scrub_finish

We don't want to change missing sets during a chunky
scrub since it would cause !is_clean() and derail
the rest of the scrub.  Instead, move the missing,
inconsistent, and authoritative sets into scrubber
and add to during scrub_compare_maps().  Then,
handle repairing objects all at once in scrub_finish().

Signed-off-by: Samuel Just <sam.just@inktank.com>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index e61261f48b8a10c412424d8b58623eba6a959256..169a5bdf6ae803cfb093f55044e21cb17ec71543 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -3423,12 +3423,6 @@ void PG::repair_object(const hobject_t& soid, ScrubMap::object *po, int bad_peer
 
     log.last_requested = 0;
   }
-  queue_peering_event(
-    CephPeeringEvtRef(
-      new CephPeeringEvt(
-        get_osdmap()->get_epoch(),
-       get_osdmap()->get_epoch(),
-       DoRecovery())));
 }
 
 /* replica_scrub
@@ -4136,18 +4130,11 @@ void PG::_compare_scrubmaps(const map<int,ScrubMap*> &maps,
 
 void PG::scrub_compare_maps() {
   dout(10) << "scrub_compare_maps has maps, analyzing" << dendl;
-  bool repair = state_test(PG_STATE_REPAIR);
-  bool deep_scrub = state_test(PG_STATE_DEEP_SCRUB);
-  const char *mode = (repair ? "repair": (deep_scrub ? "deep-scrub" : "scrub"));
   if (acting.size() > 1) {
     dout(10) << "scrub  comparing replica scrub maps" << dendl;
 
     stringstream ss;
 
-    // Maps from objects with erros to missing/inconsistent peers
-    map<hobject_t, set<int> > missing;
-    map<hobject_t, set<int> > inconsistent;
-
     // Map from object with errors to good peer
     map<hobject_t, int> authoritative;
     map<int,ScrubMap *> maps;
@@ -4161,46 +4148,24 @@ void PG::scrub_compare_maps() {
       maps[i] = &scrubber.received_maps[acting[i]];
     }
 
-    _compare_scrubmaps(maps, missing, inconsistent, authoritative, ss);
+    _compare_scrubmaps(
+      maps,
+      scrubber.missing,
+      scrubber.inconsistent,
+      authoritative,
+      ss);
+    dout(2) << ss.str() << dendl;
 
-    if (authoritative.size()) {
-      ss << info.pgid << " " << mode << " " << missing.size() << " missing, "
-        << inconsistent.size() << " inconsistent objects\n";
-      dout(2) << ss.str() << dendl;
+    if (authoritative.size())
       osd->clog.error(ss);
-      state_set(PG_STATE_INCONSISTENT);
-      if (repair) {
-       state_clear(PG_STATE_CLEAN);
-       for (map<hobject_t, int>::iterator i = authoritative.begin();
-            i != authoritative.end();
-            i++) {
-         set<int>::iterator j;
-         
-         if (missing.count(i->first)) {
-           for (j = missing[i->first].begin();
-                j != missing[i->first].end(); 
-                j++) {
-             repair_object(i->first, 
-                           &maps[i->second]->objects[i->first],
-                           acting[*j],
-                           acting[i->second]);
-              ++scrubber.fixed;
-           }
-         }
-         if (inconsistent.count(i->first)) {
-           for (j = inconsistent[i->first].begin(); 
-                j != inconsistent[i->first].end(); 
-                j++) {
-             repair_object(i->first, 
-                           &maps[i->second]->objects[i->first],
-                           acting[*j],
-                           acting[i->second]);
-              ++scrubber.fixed;
-           }
-         }
 
-       }
-      }
+    for (map<hobject_t, int>::iterator i = authoritative.begin();
+        i != authoritative.end();
+        ++i) {
+      scrubber.authoritative.insert(
+       make_pair(
+         i->first,
+         make_pair(maps[i->second]->objects[i->first], i->second)));
     }
   }
 
@@ -4208,6 +4173,53 @@ void PG::scrub_compare_maps() {
   _scrub(scrubber.primary_scrubmap);
 }
 
+void PG::scrub_process_inconsistent() {
+  dout(10) << "process_inconsistent() checking authoritative" << dendl;
+  bool repair = state_test(PG_STATE_REPAIR);
+  bool deep_scrub = state_test(PG_STATE_DEEP_SCRUB);
+  const char *mode = (repair ? "repair": (deep_scrub ? "deep-scrub" : "scrub"));
+  if (scrubber.authoritative.size()) {
+    stringstream ss;
+    ss << info.pgid << " " << mode << " " << scrubber.missing.size() << " missing, "
+       << scrubber.inconsistent.size() << " inconsistent objects\n";
+    dout(2) << ss.str() << dendl;
+    osd->clog.error(ss);
+    state_set(PG_STATE_INCONSISTENT);
+    if (repair) {
+      state_clear(PG_STATE_CLEAN);
+      for (map<hobject_t, pair<ScrubMap::object, int> >::iterator i =
+            scrubber.authoritative.begin();
+          i != scrubber.authoritative.end();
+          i++) {
+       set<int>::iterator j;
+       
+       if (scrubber.missing.count(i->first)) {
+         for (j = scrubber.missing[i->first].begin();
+              j != scrubber.missing[i->first].end(); 
+              j++) {
+           repair_object(i->first, 
+             &(i->second.first),
+             acting[*j],
+             acting[i->second.second]);
+           ++scrubber.fixed;
+         }
+       }
+       if (scrubber.inconsistent.count(i->first)) {
+         for (j = scrubber.inconsistent[i->first].begin(); 
+              j != scrubber.inconsistent[i->first].end(); 
+              j++) {
+           repair_object(i->first, 
+             &(i->second.first),
+             acting[*j],
+             acting[i->second.second]);
+           ++scrubber.fixed;
+         }
+       }
+      }
+    }
+  }
+}
+
 void PG::scrub_finalize() {
   lock();
   if (deleting) {
@@ -4248,6 +4260,8 @@ void PG::scrub_finish() {
   // type-specific finish (can tally more errors)
   _scrub_finish();
 
+  scrub_process_inconsistent();
+
   if (scrubber.errors == 0 || (repair && (scrubber.errors - scrubber.fixed) == 0))
     state_clear(PG_STATE_INCONSISTENT);
 
@@ -4284,6 +4298,16 @@ void PG::scrub_finish() {
     assert(tr == 0);
   }
 
+
+  if (scrubber.fixed) {
+    queue_peering_event(
+      CephPeeringEvtRef(
+       new CephPeeringEvt(
+         get_osdmap()->get_epoch(),
+         get_osdmap()->get_epoch(),
+         DoRecovery())));
+  }
+
   scrub_clear_state();
   scrub_unreserve_replicas();
 

diff --git a/src/osd/PG.h b/src/osd/PG.h
index e66b45b2f769015f362907d68db31987c6186a7e..bcd179d47273d7b1ccca5bff456e6edc29dfebf6 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -836,6 +836,13 @@ public:
     map<int,ScrubMap> received_maps;
     MOSDRepScrub *active_rep_scrub;
 
+    // Maps from objects with erros to missing/inconsistent peers
+    map<hobject_t, set<int> > missing;
+    map<hobject_t, set<int> > inconsistent;
+
+    // Map from object with errors to good peer
+    map<hobject_t, pair<ScrubMap::object, int> > authoritative;
+
     // classic scrub
     bool finalizing;
 
@@ -928,6 +935,9 @@ public:
       fixed = 0;
       deep = false;
       run_callbacks();
+      inconsistent.clear();
+      missing.clear();
+      authoritative.clear();
     }
 
   } scrubber;
@@ -947,6 +957,7 @@ public:
   void classic_scrub();
   void chunky_scrub();
   void scrub_compare_maps();
+  void scrub_process_inconsistent();
   void scrub_finalize();
   void scrub_finish();
   void scrub_clear_state();