From c789fd1899f697a994a28e45a6f9262c6721d77d Mon Sep 17 00:00:00 2001 From: David Zafman Date: Tue, 31 Jul 2018 17:53:17 -0700 Subject: [PATCH] osd: Move object error check out of be_select_auth_object() Fixes: http://tracker.ceph.com/issues/25108 Signed-off-by: David Zafman --- src/osd/PGBackend.cc | 96 +++++++++++++++++++++++++++++++------------- src/osd/PGBackend.h | 4 +- 2 files changed, 70 insertions(+), 30 deletions(-) diff --git a/src/osd/PGBackend.cc b/src/osd/PGBackend.cc index 7fa323c54fd4d..30db0842cbdd0 100644 --- a/src/osd/PGBackend.cc +++ b/src/osd/PGBackend.cc @@ -624,7 +624,8 @@ bool PGBackend::be_compare_scrub_objects( const ScrubMap::object &candidate, shard_info_wrapper &shard_result, inconsistent_obj_wrapper &obj_result, - ostream &errorstream) + ostream &errorstream, + bool has_snapset) { enum { CLEAN, FOUND_ERROR } error = CLEAN; if (candidate.stat_error) { @@ -684,6 +685,67 @@ bool PGBackend::be_compare_scrub_objects( } if (candidate.stat_error) return error == FOUND_ERROR; + if (!shard_result.has_info_missing() + && !shard_result.has_info_corrupted()) { + bufferlist can_bl, auth_bl; + auto can_attr = candidate.attrs.find(OI_ATTR); + auto auth_attr = auth.attrs.find(OI_ATTR); + + assert(auth_attr != auth.attrs.end()); + assert(can_attr != candidate.attrs.end()); + + can_bl.push_back(can_attr->second); + auth_bl.push_back(auth_attr->second); + if (!can_bl.contents_equal(auth_bl)) { + if (error != CLEAN) + errorstream << ", "; + error = FOUND_ERROR; + obj_result.set_object_info_inconsistency(); + errorstream << "object info inconsistent "; + } + } + if (has_snapset) { + if (!shard_result.has_snapset_missing() + && !shard_result.has_snapset_corrupted()) { + bufferlist can_bl, auth_bl; + auto can_attr = candidate.attrs.find(SS_ATTR); + auto auth_attr = auth.attrs.find(SS_ATTR); + + assert(auth_attr != auth.attrs.end()); + assert(can_attr != candidate.attrs.end()); + + can_bl.push_back(can_attr->second); + auth_bl.push_back(auth_attr->second); + if (!can_bl.contents_equal(auth_bl)) { + if (error != CLEAN) + errorstream << ", "; + error = FOUND_ERROR; + obj_result.set_snapset_inconsistency(); + errorstream << "snapset inconsistent "; + } + } + } + if (parent->get_pool().is_erasure()) { + if (!shard_result.has_hinfo_missing() + && !shard_result.has_hinfo_corrupted()) { + bufferlist can_bl, auth_bl; + auto can_hi = candidate.attrs.find(ECUtil::get_hinfo_key()); + auto auth_hi = auth.attrs.find(ECUtil::get_hinfo_key()); + + assert(auth_hi != auth.attrs.end()); + assert(can_hi != candidate.attrs.end()); + + can_bl.push_back(can_hi->second); + auth_bl.push_back(auth_hi->second); + if (!can_bl.contents_equal(auth_bl)) { + if (error != CLEAN) + errorstream << ", "; + error = FOUND_ERROR; + obj_result.set_hinfo_inconsistency(); + errorstream << "hinfo inconsistent "; + } + } + } uint64_t oi_size = be_get_ondisk_size(auth_oi.size); if (oi_size != candidate.size) { if (error != CLEAN) @@ -756,11 +818,9 @@ map::const_iterator const map &maps, object_info_t *auth_oi, map &shard_map, - inconsistent_obj_wrapper &object_error, bool &digest_match) { eversion_t auth_version; - bufferlist first_oi_bl, first_ss_bl, first_hk_bl; // Create list of shards with primary first so it will be auth copy all // other things being equal. @@ -826,12 +886,6 @@ map::const_iterator try { auto bliter = ss_bl.cbegin(); decode(ss, bliter); - if (first_ss_bl.length() == 0) { - first_ss_bl.append(ss_bl); - } else if (!object_error.has_snapset_inconsistency() && !ss_bl.contents_equal(first_ss_bl)) { - object_error.set_snapset_inconsistency(); - error_string += " snapset_inconsistency"; - } } catch (...) { // invalid snapset, probably corrupt shard_info.set_snapset_corrupted(); @@ -851,12 +905,6 @@ map::const_iterator try { auto bliter = hk_bl.cbegin(); decode(hi, bliter); - if (first_hk_bl.length() == 0) { - first_hk_bl.append(hk_bl); - } else if (!object_error.has_hinfo_inconsistency() && !hk_bl.contents_equal(first_hk_bl)) { - object_error.set_hinfo_inconsistency(); - error_string += " hinfo_inconsistency"; - } } catch (...) { // invalid snapset, probably corrupt shard_info.set_hinfo_corrupted(); @@ -886,13 +934,6 @@ map::const_iterator // This is automatically corrected in PG::_repair_oinfo_oid() assert(oi.soid == obj); - if (first_oi_bl.length() == 0) { - first_oi_bl.append(bl); - } else if (!object_error.has_object_info_inconsistency() && !bl.contents_equal(first_oi_bl)) { - object_error.set_object_info_inconsistency(); - error_string += " object_info_inconsistency"; - } - if (i->second.size != be_get_ondisk_size(oi.size)) { dout(5) << __func__ << " size " << i->second.size << " oi size " << oi.size << dendl; shard_info.set_obj_size_info_mismatch(); @@ -967,8 +1008,7 @@ void PGBackend::be_compare_scrubmaps( bool digest_match; map::const_iterator auth = - be_select_auth_object(*k, maps, &auth_oi, shard_map, object_error, - digest_match); + be_select_auth_object(*k, maps, &auth_oi, shard_map, digest_match); list auth_list; set object_errors; @@ -1004,7 +1044,8 @@ void PGBackend::be_compare_scrubmaps( j->second->objects[*k], shard_map[j->first], object_error, - ss); + ss, + k->has_snapset()); dout(20) << __func__ << (repair ? " repair " : " ") << (parent->get_pool().is_replicated() ? "replicated " : "") << (j == auth ? "auth" : "") << "shards " << shard_map.size() << (digest_match ? " digest_match " : " ") @@ -1033,12 +1074,11 @@ void PGBackend::be_compare_scrubmaps( if (found) errorstream << pgid << " shard " << j->first << ": soid " << *k << " " << ss.str() << "\n"; - } else if (object_error.errors != 0) { + } else if (found) { // Track possible shard to use as authoritative, if needed // There are errors, without identifying the shard object_errors.insert(j->first); - if (found) - errorstream << pgid << " : soid " << *k << " " << ss.str() << "\n"; + errorstream << pgid << " : soid " << *k << " " << ss.str() << "\n"; } else { // XXX: The auth shard might get here that we don't know // that it has the "correct" data. diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h index 957f965907b1f..3134540c5131d 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -570,13 +570,13 @@ typedef std::shared_ptr OSDMapRef; const ScrubMap::object &candidate, shard_info_wrapper& shard_error, inconsistent_obj_wrapper &result, - ostream &errorstream); + ostream &errorstream, + bool has_snapset); map::const_iterator be_select_auth_object( const hobject_t &obj, const map &maps, object_info_t *auth_oi, map &shard_map, - inconsistent_obj_wrapper &object_error, bool &digest_match); void be_compare_scrubmaps( const map &maps, -- 2.39.5