osd: handle case when stat returns with error in get_hash_info

Previously in the case of the error we stored in the cache and
returned HashInfo(ec_impl->get_chunk_count()), which e.g. could
propagate to non-primary shards, introducing inconsistency.

The function's `checks` flag is replaced with `create` flag,
which seems to have more clear meaning here.

In be_deep_scrub the get_hash_info is still called with the
second argument false (i.e. with `create=false`, while previously
it was `checks=false`), which is done intentionally.

Fixes: https://tracker.ceph.com/issues/48959
Signed-off-by: Mykola Golub <mgolub@suse.com>
This commit is contained in:
Mykola Golub 2021-09-21 16:17:17 +01:00
parent 104d8df5f4
commit e303dc1cf5
2 changed files with 8 additions and 5 deletions

View File

@ -1827,7 +1827,7 @@ void ECBackend::do_read_op(ReadOp &op)
}
ECUtil::HashInfoRef ECBackend::get_hash_info(
const hobject_t &hoid, bool checks, const map<string,bufferptr,less<>> *attrs)
const hobject_t &hoid, bool create, const map<string,bufferptr,less<>> *attrs)
{
dout(10) << __func__ << ": Getting attr on " << hoid << dendl;
ECUtil::HashInfoRef ref = unstable_hashinfo_registry.lookup(hoid);
@ -1839,7 +1839,6 @@ ECUtil::HashInfoRef ECBackend::get_hash_info(
ghobject_t(hoid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard),
&st);
ECUtil::HashInfo hinfo(ec_impl->get_chunk_count());
// XXX: What does it mean if there is no object on disk?
if (r >= 0) {
dout(10) << __func__ << ": found on disk, size " << st.st_size << dendl;
bufferlist bl;
@ -1869,7 +1868,7 @@ ECUtil::HashInfoRef ECBackend::get_hash_info(
dout(0) << __func__ << ": Can't decode hinfo for " << hoid << dendl;
return ECUtil::HashInfoRef();
}
if (checks && hinfo.get_total_chunk_size() != (uint64_t)st.st_size) {
if (hinfo.get_total_chunk_size() != (uint64_t)st.st_size) {
dout(0) << __func__ << ": Mismatch of total_chunk_size "
<< hinfo.get_total_chunk_size() << dendl;
return ECUtil::HashInfoRef();
@ -1877,6 +1876,10 @@ ECUtil::HashInfoRef ECBackend::get_hash_info(
} else if (st.st_size > 0) { // If empty object and no hinfo, create it
return ECUtil::HashInfoRef();
}
} else if (r != -ENOENT || !create) {
derr << __func__ << ": stat " << hoid << " failed: " << cpp_strerror(r)
<< dendl;
return ECUtil::HashInfoRef();
}
ref = unstable_hashinfo_registry.lookup_or_create(hoid, hinfo);
}
@ -1891,7 +1894,7 @@ void ECBackend::start_rmw(Op *op, PGTransactionUPtr &&t)
sinfo,
std::move(t),
[&](const hobject_t &i) {
ECUtil::HashInfoRef ref = get_hash_info(i, false);
ECUtil::HashInfoRef ref = get_hash_info(i, true);
if (!ref) {
derr << __func__ << ": get_hash_info(" << i << ")"
<< " returned a null pointer and there is no "

View File

@ -629,7 +629,7 @@ public:
const ECUtil::stripe_info_t sinfo;
/// If modified, ensure that the ref is held until the update is applied
SharedPtrRegistry<hobject_t, ECUtil::HashInfo> unstable_hashinfo_registry;
ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, bool checks = true,
ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, bool create = false,
const std::map<std::string, ceph::buffer::ptr, std::less<>> *attr = NULL);
public: