From 3f10311b7a87118dc63c8092b9ecbf4df8c71c96 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Rados=C5=82aw=20Zarzy=C5=84ski?= Date: Wed, 8 Nov 2023 18:41:19 +0100 Subject: [PATCH] osd: ECBackend::get_hash_info() takes external object size MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This allows to reduce the interactions with `ObjectStore` by letting callers `get_hash_info()` to provide the object size from external source, from a cache in partiular. The same idea will be reused in crimson but with an extra benefit: no need for green threads within `submit_transaction()` of `ECBackend`. Signed-off-by: Radosław Zarzyński (cherry picked from commit 98347a6013c9244c8768b1e570d36d0e17cd45d3) --- src/osd/ECBackend.cc | 94 +++++++++++++++++++++++++------------------- src/osd/ECBackend.h | 8 +++- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc index 686e179f987..8847d5b0907 100644 --- a/src/osd/ECBackend.cc +++ b/src/osd/ECBackend.cc @@ -518,7 +518,7 @@ void ECBackend::continue_recovery_op( if (op.recovery_progress.first && op.obc) { /* We've got the attrs and the hinfo, might as well use them */ - op.hinfo = get_hash_info(op.hoid, false, op.obc->attr_cache); + op.hinfo = get_hash_info(op.hoid, false, op.obc->attr_cache, op.obc->obs.oi.size); if (!op.hinfo) { derr << __func__ << ": " << op.hoid << " has inconsistent hinfo" << dendl; @@ -1013,12 +1013,17 @@ void ECBackend::handle_sub_read( // the state of our chunk in case other chunks could substitute. ECUtil::HashInfoRef hinfo; map> attrs; - int r = PGBackend::objects_get_attrs(i->first, &attrs); + struct stat st; + int r = object_stat(i->first, &st); if (r >= 0) { - hinfo = get_hash_info(i->first, false, attrs); + dout(10) << __func__ << ": found on disk, size " << st.st_size << dendl; + r = PGBackend::objects_get_attrs(i->first, &attrs); + } + if (r >= 0) { + hinfo = get_hash_info(i->first, false, attrs, st.st_size); } else { - derr << "get_hash_info" << ": stat " << i->first << " failed: " - << cpp_strerror(r) << dendl; + derr << __func__ << ": access (attrs) on " << i->first << " failed: " + << cpp_strerror(r) << dendl; } if (!hinfo) { r = -EIO; @@ -1377,7 +1382,11 @@ void ECBackend::submit_transaction( sinfo, *(op->t), [&](const hobject_t &i) { - ECUtil::HashInfoRef ref = get_hash_info(i, true, op->t->obc_map[hoid]->attr_cache); + ECUtil::HashInfoRef ref = get_hash_info( + i, + true, + op->t->obc_map[hoid]->attr_cache, + op->t->obc_map[hoid]->obs.oi.size); if (!ref) { derr << __func__ << ": get_hash_info(" << i << ")" << " returned a null pointer and there is no " @@ -1394,49 +1403,42 @@ void ECBackend::submit_transaction( ECUtil::HashInfoRef ECBackend::get_hash_info( - const hobject_t &hoid, bool create, const map>& attrs) + const hobject_t &hoid, + bool create, + const map>& attrs, + uint64_t size) { dout(10) << __func__ << ": Getting attr on " << hoid << dendl; ECUtil::HashInfoRef ref = unstable_hashinfo_registry.lookup(hoid); if (!ref) { dout(10) << __func__ << ": not in cache " << hoid << dendl; - struct stat st; - int r = store->stat( - ch, - ghobject_t(hoid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard), - &st); ECUtil::HashInfo hinfo(ec_impl->get_chunk_count()); - if (r >= 0) { - dout(10) << __func__ << ": found on disk, size " << st.st_size << dendl; - bufferlist bl; - map::const_iterator k = attrs.find(ECUtil::get_hinfo_key()); - if (k == attrs.end()) { - dout(5) << __func__ << " " << hoid << " missing hinfo attr" << dendl; - } else { - bl = k->second; + bufferlist bl; + map::const_iterator k = attrs.find(ECUtil::get_hinfo_key()); + if (k == attrs.end()) { + dout(5) << __func__ << " " << hoid << " missing hinfo attr" << dendl; + } else { + bl = k->second; + } + if (bl.length() > 0) { + auto bp = bl.cbegin(); + try { + decode(hinfo, bp); + } catch(...) { + dout(0) << __func__ << ": Can't decode hinfo for " << hoid << dendl; + return ECUtil::HashInfoRef(); } - if (bl.length() > 0) { - auto bp = bl.cbegin(); - try { - decode(hinfo, bp); - } catch(...) { - dout(0) << __func__ << ": Can't decode hinfo for " << hoid << dendl; - return ECUtil::HashInfoRef(); - } - 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(); - } - } else if (st.st_size > 0) { // If empty object and no hinfo, create it - return ECUtil::HashInfoRef(); + if (hinfo.get_total_chunk_size() != size) { + dout(0) << __func__ << ": Mismatch of total_chunk_size " + << hinfo.get_total_chunk_size() << dendl; + return ECUtil::HashInfoRef(); } - } else if (r != -ENOENT || !create) { - derr << __func__ << ": stat " << hoid << " failed: " << cpp_strerror(r) - << dendl; - return ECUtil::HashInfoRef(); + } else if (size == 0) { // If empty object and no hinfo, create it + create = true; + } + if (create) { + ref = unstable_hashinfo_registry.lookup_or_create(hoid, hinfo); } - ref = unstable_hashinfo_registry.lookup_or_create(hoid, hinfo); } return ref; } @@ -1586,6 +1588,16 @@ void ECBackend::kick_reads() { read_pipeline.kick_reads(); } +int ECBackend::object_stat( + const hobject_t &hoid, + struct stat* st) +{ + int r = store->stat( + ch, + ghobject_t{hoid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard}, + st); + return r; +} int ECBackend::objects_get_attrs( const hobject_t &hoid, @@ -1676,7 +1688,7 @@ int ECBackend::be_deep_scrub( return -EINPROGRESS; } - ECUtil::HashInfoRef hinfo = get_hash_info(poid, false, o.attrs); + ECUtil::HashInfoRef hinfo = get_hash_info(poid, false, o.attrs, o.size); if (!hinfo) { dout(0) << "_scan_list " << poid << " could not retrieve hash info" << dendl; o.read_error = true; diff --git a/src/osd/ECBackend.h b/src/osd/ECBackend.h index a134ea58e69..0b3d79d8bf0 100644 --- a/src/osd/ECBackend.h +++ b/src/osd/ECBackend.h @@ -325,8 +325,12 @@ public: const ECUtil::stripe_info_t sinfo; /// If modified, ensure that the ref is held until the update is applied SharedPtrRegistry unstable_hashinfo_registry; - ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, bool create, - const std::map>& attr); + ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, + bool create, + const std::map>& attr, + uint64_t size); + + int object_stat(const hobject_t &hoid, struct stat* st); public: ECBackend( -- 2.39.5