osd: ECBackend::get_hash_info() takes external object size

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 <rzarzyns@redhat.com>
(cherry picked from commit 98347a6013c9244c8768b1e570d36d0e17cd45d3)

diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
index 686e179f987a253e4171dc0e716c9bb58dd74290..8847d5b090735ed77041c9e2ab9e13287bf8c0fa 100644 (file)
--- 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<string, bufferlist, less<>> 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<string,bufferlist,less<>>& attrs)
+  const hobject_t &hoid,
+  bool create,
+  const map<string, bufferlist, less<>>& 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<string, bufferlist>::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<string, bufferlist>::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 a134ea58e695e53620761e7a9599048dfbddb204..0b3d79d8bf0ab2962eca1bc7358808fd79294a38 100644 (file)
--- 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<hobject_t, ECUtil::HashInfo> unstable_hashinfo_registry;
-  ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid, bool create,
-                                   const std::map<std::string, ceph::buffer::list, std::less<>>& attr);
+  ECUtil::HashInfoRef get_hash_info(const hobject_t &hoid,
+                                   bool create,
+                                   const std::map<std::string, ceph::buffer::list, std::less<>>& attr,
+                                   uint64_t size);
+
+  int object_stat(const hobject_t &hoid, struct stat* st);
 
 public:
   ECBackend(