From: David Zafman Date: Tue, 23 Apr 2013 00:06:52 +0000 (-0700) Subject: scrub clears inconsistent flag set by deep scrub X-Git-Tag: v0.61~101^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ac3dda214d52c10206328a92e4373521200c8863;p=ceph.git 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 --- diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 8cab7c774008..7875a240ca40 100644 --- 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::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 &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::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 f819e46c9637..48636476812b 100644 --- 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 waiting_on_whom; - int errors; + int shallow_errors; + int deep_errors; int fixed; ScrubMap primary_scrubmap; map 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 > missing; map > inconsistent; map > 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::const_iterator _select_auth_object( const hobject_t &obj, const map &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 &maps, diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index 90f6bf5c18ea..a76a441eed59 100644 --- 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 83c688707650..62e2ba73fa4c 100644 --- 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& 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 12a410ef2d22..7a1937e5e406 100644 --- 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) {}