scrub clears inconsistent flag set by deep scrub

Add new num_deep_scrub_errors and num_shallow_scrub_errors to object_stat_sum_t
Show deep-scrub error count when outputing regular scrub errors
Set invalid size in case of a stat error which sets read_error
For now do deep-scrub after repair (see #4783)

fixes: #4778
Signed-off-by: David Zafman <david.zafman@inktank.com>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 8cab7c774008f3d16b5a1f1d8b44b3076427a2c2..7875a240ca40fa3dc9354e6073147e8b7c3b9a88 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -1995,6 +1995,7 @@ void PG::_finish_recovery(Context *c)
     if (scrub_after_recovery) {
       dout(10) << "_finish_recovery requeueing for scrub" << dendl;
       scrub_after_recovery = false;
+      scrubber.must_deep_scrub = true;
       queue_scrub();
     }
   } else {
@@ -4278,25 +4279,22 @@ bool PG::scrub_gather_replica_maps() {
   }
 }
 
-bool PG::_compare_scrub_objects(ScrubMap::object &auth,
+enum PG::error_type PG::_compare_scrub_objects(ScrubMap::object &auth,
                                ScrubMap::object &candidate,
                                ostream &errorstream)
 {
-  bool ok = true;
+  enum PG::error_type error = CLEAN;
   if (candidate.read_error) {
-    ok = false;
+    // This can occur on stat() of a shallow scrub, but in that case size will
+    // be invalid, and this will be over-ridden below.
+    error = DEEP_ERROR;
     errorstream << "candidate had a read error";
   }
-  if (auth.size != candidate.size) {
-    ok = false;
-    errorstream << "size " << candidate.size 
-               << " != known size " << auth.size;
-  }
   if (auth.digest_present && candidate.digest_present) {
     if (auth.digest != candidate.digest) {
-      if (!ok)
+      if (error != CLEAN)
         errorstream << ", ";
-      ok = false;
+      error = DEEP_ERROR;
 
       errorstream << "digest " << candidate.digest
                   << " != known digest " << auth.digest;
@@ -4304,26 +4302,35 @@ bool PG::_compare_scrub_objects(ScrubMap::object &auth,
   }
   if (auth.omap_digest_present && candidate.omap_digest_present) {
     if (auth.omap_digest != candidate.omap_digest) {
-      if (!ok)
+      if (error != CLEAN)
         errorstream << ", ";
-      ok = false;
+      error = DEEP_ERROR;
 
       errorstream << "omap_digest " << candidate.omap_digest
                   << " != known omap_digest " << auth.omap_digest;
     }
   }
+  // Shallow error takes precendence because this will be seen by
+  // both types of scrubs.
+  if (auth.size != candidate.size) {
+    if (error != CLEAN)
+      errorstream << ", ";
+    error = SHALLOW_ERROR;
+    errorstream << "size " << candidate.size 
+               << " != known size " << auth.size;
+  }
   for (map<string,bufferptr>::const_iterator i = auth.attrs.begin();
        i != auth.attrs.end();
        ++i) {
     if (!candidate.attrs.count(i->first)) {
-      if (!ok)
-       errorstream << ", ";
-      ok = false;
+      if (error != CLEAN)
+        errorstream << ", ";
+      error = SHALLOW_ERROR;
       errorstream << "missing attr " << i->first;
     } else if (candidate.attrs.find(i->first)->second.cmp(i->second)) {
-      if (!ok)
-       errorstream << ", ";
-      ok = false;
+      if (error != CLEAN)
+        errorstream << ", ";
+      error = SHALLOW_ERROR;
       errorstream << "attr value mismatch " << i->first;
     }
   }
@@ -4331,13 +4338,13 @@ bool PG::_compare_scrub_objects(ScrubMap::object &auth,
        i != candidate.attrs.end();
        ++i) {
     if (!auth.attrs.count(i->first)) {
-      if (!ok)
-       errorstream << ", ";
-      ok = false;
+      if (error != CLEAN)
+        errorstream << ", ";
+      error = SHALLOW_ERROR;
       errorstream << "extra attr " << i->first;
     }
   }
-  return ok;
+  return error;
 }
 
 
@@ -4446,17 +4453,21 @@ void PG::_compare_scrubmaps(const map<int,ScrubMap*> &maps,
       if (j->second->objects.count(*k)) {
        // Compare
        stringstream ss;
-       if (!_compare_scrub_objects(auth->second->objects[*k],
+       enum PG::error_type error = _compare_scrub_objects(auth->second->objects[*k],
            j->second->objects[*k],
-           ss)) {
+           ss);
+        if (error != CLEAN) {
          cur_inconsistent.insert(j->first);
-         ++scrubber.errors;
+          if (error == SHALLOW_ERROR)
+           ++scrubber.shallow_errors;
+          else
+           ++scrubber.deep_errors;
          errorstream << info.pgid << " osd." << acting[j->first]
                      << ": soid " << *k << " " << ss.str() << std::endl;
        }
       } else {
        cur_missing.insert(j->first);
-       ++scrubber.errors;
+       ++scrubber.shallow_errors;
        errorstream << info.pgid
                    << " osd." << acting[j->first] 
                    << " missing " << *k << std::endl;
@@ -4548,7 +4559,7 @@ void PG::scrub_process_inconsistent() {
       for (set<int>::iterator j = obj->second.begin();
           j != obj->second.end();
           ++j) {
-       ++scrubber.errors;
+       ++scrubber.shallow_errors;
        ss << info.pgid << " " << mode << " " << " object " << obj->first
           << " has inconsistent snapcolls on " << *j << std::endl;
       }
@@ -4639,14 +4650,18 @@ void PG::scrub_finish() {
   {
     stringstream oss;
     oss << info.pgid << " " << mode << " ";
-    if (scrubber.errors)
-      oss << scrubber.errors << " errors";
+    int total_errors = scrubber.shallow_errors + scrubber.deep_errors;
+    if (total_errors)
+      oss << total_errors << " errors";
     else
       oss << "ok";
+    if (!deep_scrub && info.stats.stats.sum.num_deep_scrub_errors)
+      oss << " ( " << info.stats.stats.sum.num_deep_scrub_errors
+          << " remaining deep scrub error(s) )";
     if (repair)
       oss << ", " << scrubber.fixed << " fixed";
     oss << "\n";
-    if (scrubber.errors)
+    if (total_errors)
       osd->clog.error(oss);
     else
       osd->clog.info(oss);
@@ -4661,9 +4676,21 @@ void PG::scrub_finish() {
     info.history.last_deep_scrub = info.last_update;
     info.history.last_deep_scrub_stamp = now;
   }
-  if (scrubber.errors == 0)
-    info.history.last_clean_scrub_stamp = now;
-  info.stats.stats.sum.num_scrub_errors = scrubber.errors;
+  if (deep_scrub) {
+    if ((scrubber.shallow_errors == 0) && (scrubber.deep_errors == 0))
+      info.history.last_clean_scrub_stamp = now;
+    info.stats.stats.sum.num_shallow_scrub_errors = scrubber.shallow_errors;
+    info.stats.stats.sum.num_deep_scrub_errors = scrubber.deep_errors;
+  } else {
+    info.stats.stats.sum.num_shallow_scrub_errors = scrubber.shallow_errors;
+    // XXX: last_clean_scrub_stamp doesn't mean the pg is not inconsistent
+    // because of deep-scrub errors
+    if (scrubber.shallow_errors == 0)
+      info.history.last_clean_scrub_stamp = now;
+  }
+  info.stats.stats.sum.num_scrub_errors = 
+    info.stats.stats.sum.num_shallow_scrub_errors +
+    info.stats.stats.sum.num_deep_scrub_errors;
   reg_next_scrub();
 
   {

diff --git a/src/osd/PG.h b/src/osd/PG.h
index f819e46c9637b45990cf666e67290f503b5d325a..48636476812be495aa8137b992678a2b6127fd33 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -864,7 +864,8 @@ public:
       reserved(false), reserve_failed(false),
       epoch_start(0),
       block_writes(false), active(false), queue_snap_trim(false),
-      waiting_on(0), errors(0), fixed(0), active_rep_scrub(0),
+      waiting_on(0), shallow_errors(0), deep_errors(0), fixed(0),
+      active_rep_scrub(0),
       must_scrub(false), must_deep_scrub(false), must_repair(false),
       classic(false),
       finalizing(false), is_chunky(false), state(INACTIVE),
@@ -883,7 +884,8 @@ public:
     bool queue_snap_trim;
     int waiting_on;
     set<int> waiting_on_whom;
-    int errors;
+    int shallow_errors;
+    int deep_errors;
     int fixed;
     ScrubMap primary_scrubmap;
     map<int,ScrubMap> received_maps;
@@ -893,7 +895,7 @@ public:
     // flags to indicate explicitly requested scrubs (by admin)
     bool must_scrub, must_deep_scrub, must_repair;
 
-    // Maps from objects with erros to missing/inconsistent peers
+    // Maps from objects with errors to missing/inconsistent peers
     map<hobject_t, set<int> > missing;
     map<hobject_t, set<int> > inconsistent;
     map<hobject_t, set<int> > inconsistent_snapcolls;
@@ -995,7 +997,8 @@ public:
       start = hobject_t();
       end = hobject_t();
       subset_last_update = eversion_t();
-      errors = 0;
+      shallow_errors = 0;
+      deep_errors = 0;
       fixed = 0;
       deep = false;
       run_callbacks();
@@ -1014,7 +1017,13 @@ public:
   map<int, ScrubMap *>::const_iterator _select_auth_object(
     const hobject_t &obj,
     const map<int,ScrubMap*> &maps);
-  bool _compare_scrub_objects(ScrubMap::object &auth,
+
+  enum error_type {
+    CLEAN,
+    DEEP_ERROR,
+    SHALLOW_ERROR
+  };
+  enum error_type _compare_scrub_objects(ScrubMap::object &auth,
                              ScrubMap::object &candidate,
                              ostream &errorstream);
   void _compare_scrubmaps(const map<int,ScrubMap*> &maps,  

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 90f6bf5c18ea430e3f5757aef1d6ceca04dbea5c..a76a441eed59af46a0ec8e5449f0ab204fb76b5c 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -7125,7 +7125,7 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
       if (p->second.attrs.count(SS_ATTR) == 0) {
        osd->clog.error() << mode << " " << info.pgid << " " << soid
                          << " no '" << SS_ATTR << "' attr";
-        ++scrubber.errors;
+        ++scrubber.shallow_errors;
        continue;
       }
       bufferlist bl;
@@ -7137,7 +7137,7 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
       if (head != hobject_t()) {
        osd->clog.error() << mode << " " << info.pgid << " " << head
                          << " missing clones";
-        ++scrubber.errors;
+        ++scrubber.shallow_errors;
       }
       
       // what will be next?
@@ -7170,7 +7170,7 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
     if (p->second.attrs.count(OI_ATTR) == 0) {
       osd->clog.error() << mode << " " << info.pgid << " " << soid
                        << " no '" << OI_ATTR << "' attr";
-      ++scrubber.errors;
+      ++scrubber.shallow_errors;
       continue;
     }
     bufferlist bv;
@@ -7181,7 +7181,7 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
       osd->clog.error() << mode << " " << info.pgid << " " << soid
                        << " on disk size (" << p->second.size
                        << ") does not match object info size (" << oi.size << ")";
-      ++scrubber.errors;
+      ++scrubber.shallow_errors;
     }
 
     dout(20) << mode << "  " << soid << " " << oi << dendl;
@@ -7197,7 +7197,7 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
       if (!snapset.head_exists) {
        osd->clog.error() << mode << " " << info.pgid << " " << soid
                          << " snapset.head_exists=false, but object exists";
-        ++scrubber.errors;
+        ++scrubber.shallow_errors;
        continue;
       }
     } else if (soid.snap) {
@@ -7207,14 +7207,14 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
       if (head == hobject_t()) {
        osd->clog.error() << mode << " " << info.pgid << " " << soid
                          << " found clone without head";
-       ++scrubber.errors;
+       ++scrubber.shallow_errors;
        continue;
       }
 
       if (soid.snap != *curclone) {
        osd->clog.error() << mode << " " << info.pgid << " " << soid
                          << " expected clone " << *curclone;
-        ++scrubber.errors;
+        ++scrubber.shallow_errors;
        assert(soid.snap == *curclone);
       }
 
@@ -7271,7 +7271,7 @@ void ReplicatedPG::_scrub_finish()
                      << scrub_cstat.sum.num_objects << "/" << info.stats.stats.sum.num_objects << " objects, "
                      << scrub_cstat.sum.num_object_clones << "/" << info.stats.stats.sum.num_object_clones << " clones, "
                      << scrub_cstat.sum.num_bytes << "/" << info.stats.stats.sum.num_bytes << " bytes.\n";
-    ++scrubber.errors;
+    ++scrubber.shallow_errors;
 
     if (repair) {
       ++scrubber.fixed;

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index 83c688707650d169b6aac40180160007500a9de5..62e2ba73fa4ce8664bb4f5c2cd162c33a203ca18 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -911,6 +911,8 @@ void object_stat_sum_t::dump(Formatter *f) const
   f->dump_int("num_write", num_wr);
   f->dump_int("num_write_kb", num_wr_kb);
   f->dump_int("num_scrub_errors", num_scrub_errors);
+  f->dump_int("num_shallow_scrub_errors", num_shallow_scrub_errors);
+  f->dump_int("num_deep_scrub_errors", num_deep_scrub_errors);
   f->dump_int("num_objects_recovered", num_objects_recovered);
   f->dump_int("num_bytes_recovered", num_bytes_recovered);
   f->dump_int("num_keys_recovered", num_keys_recovered);
@@ -918,7 +920,7 @@ void object_stat_sum_t::dump(Formatter *f) const
 
 void object_stat_sum_t::encode(bufferlist& bl) const
 {
-  ENCODE_START(5, 3, bl);
+  ENCODE_START(6, 3, bl);
   ::encode(num_bytes, bl);
   ::encode(num_objects, bl);
   ::encode(num_object_clones, bl);
@@ -934,12 +936,14 @@ void object_stat_sum_t::encode(bufferlist& bl) const
   ::encode(num_objects_recovered, bl);
   ::encode(num_bytes_recovered, bl);
   ::encode(num_keys_recovered, bl);
+  ::encode(num_shallow_scrub_errors, bl);
+  ::encode(num_deep_scrub_errors, bl);
   ENCODE_FINISH(bl);
 }
 
 void object_stat_sum_t::decode(bufferlist::iterator& bl)
 {
-  DECODE_START_LEGACY_COMPAT_LEN(5, 3, 3, bl);
+  DECODE_START_LEGACY_COMPAT_LEN(6, 3, 3, bl);
   ::decode(num_bytes, bl);
   if (struct_v < 3) {
     uint64_t num_kb;
@@ -969,6 +973,13 @@ void object_stat_sum_t::decode(bufferlist::iterator& bl)
     num_bytes_recovered = 0;
     num_keys_recovered = 0;
   }
+  if (struct_v >= 6) {
+    ::decode(num_shallow_scrub_errors, bl);
+    ::decode(num_deep_scrub_errors, bl);
+  } else {
+    num_shallow_scrub_errors = 0;
+    num_deep_scrub_errors = 0;
+  }
   DECODE_FINISH(bl);
 }
 
@@ -986,10 +997,12 @@ void object_stat_sum_t::generate_test_instances(list<object_stat_sum_t*>& o)
   a.num_objects_unfound = 8;
   a.num_rd = 9; a.num_rd_kb = 10;
   a.num_wr = 11; a.num_wr_kb = 12;
-  a.num_scrub_errors = 13;
   a.num_objects_recovered = 14;
   a.num_bytes_recovered = 15;
   a.num_keys_recovered = 16;
+  a.num_deep_scrub_errors = 17;
+  a.num_shallow_scrub_errors = 18;
+  a.num_scrub_errors = a.num_deep_scrub_errors + a.num_shallow_scrub_errors;
   o.push_back(new object_stat_sum_t(a));
 }
 
@@ -1007,6 +1020,8 @@ void object_stat_sum_t::add(const object_stat_sum_t& o)
   num_wr_kb += o.num_wr_kb;
   num_objects_unfound += o.num_objects_unfound;
   num_scrub_errors += o.num_scrub_errors;
+  num_shallow_scrub_errors += o.num_shallow_scrub_errors;
+  num_deep_scrub_errors += o.num_deep_scrub_errors;
   num_objects_recovered += o.num_objects_recovered;
   num_bytes_recovered += o.num_bytes_recovered;
   num_keys_recovered += o.num_keys_recovered;
@@ -1026,6 +1041,8 @@ void object_stat_sum_t::sub(const object_stat_sum_t& o)
   num_wr_kb -= o.num_wr_kb;
   num_objects_unfound -= o.num_objects_unfound;
   num_scrub_errors -= o.num_scrub_errors;
+  num_shallow_scrub_errors -= o.num_shallow_scrub_errors;
+  num_deep_scrub_errors -= o.num_deep_scrub_errors;
   num_objects_recovered -= o.num_objects_recovered;
   num_bytes_recovered -= o.num_bytes_recovered;
   num_keys_recovered -= o.num_keys_recovered;

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index 12a410ef2d227525aa75468602068c3cfadd0ae3..7a1937e5e406874f47dd69f5d32b420bbb25ee41 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -800,7 +800,9 @@ struct object_stat_sum_t {
   int64_t num_objects_unfound;
   int64_t num_rd, num_rd_kb;
   int64_t num_wr, num_wr_kb;
-  int64_t num_scrub_errors;
+  int64_t num_scrub_errors;    // total deep and shallow scrub errors
+  int64_t num_shallow_scrub_errors;
+  int64_t num_deep_scrub_errors;
   int64_t num_objects_recovered;
   int64_t num_bytes_recovered;
   int64_t num_keys_recovered;
@@ -810,7 +812,8 @@ struct object_stat_sum_t {
       num_objects(0), num_object_clones(0), num_object_copies(0),
       num_objects_missing_on_primary(0), num_objects_degraded(0), num_objects_unfound(0),
       num_rd(0), num_rd_kb(0), num_wr(0), num_wr_kb(0),
-      num_scrub_errors(0),
+      num_scrub_errors(0), num_shallow_scrub_errors(0),
+      num_deep_scrub_errors(0),
       num_objects_recovered(0),
       num_bytes_recovered(0),
       num_keys_recovered(0)
@@ -1974,7 +1977,8 @@ struct ScrubMap {
     bool read_error;
 
     object() :
-      size(0), negative(false), digest(0), digest_present(false),
+      // Init invalid size so it won't match if we get a stat EIO error
+      size(-1), negative(false), digest(0), digest_present(false),
       nlinks(0), omap_digest(0), omap_digest_present(false),
       read_error(false) {}