Merge pull request ceph#56995 from athanatos/sjust/wip-65185-scrub-attr-error

osd: only call stat/getattrs once per object during deep-scrub

Reviewed-by: Radoslaw Zarzynski <[email protected]>

Author: yuriw

src/os/ObjectStore.h

@@ -617,7 +617,7 @@ class ObjectStore {
    *
    * @param cid collection for object
    * @param oid oid of object
-   * @param aset place to put output result.
+   * @param aset upon success, will contain exactly the object attrs
    * @returns 0 on success, negative error code on failure.
    */
   virtual int getattrs(CollectionHandle &c, const ghobject_t& oid,
@@ -628,13 +628,14 @@ class ObjectStore {
    *
    * @param cid collection for object
    * @param oid oid of object
-   * @param aset place to put output result.
+   * @param aset upon success, will contain exactly the object attrs
    * @returns 0 on success, negative error code on failure.
    */
   int getattrs(CollectionHandle &c, const ghobject_t& oid,
 	       std::map<std::string,ceph::buffer::list,std::less<>>& aset) {
     std::map<std::string,ceph::buffer::ptr,std::less<>> bmap;
     int r = getattrs(c, oid, bmap);
+    aset.clear();
     for (auto i = bmap.begin(); i != bmap.end(); ++i) {
       aset[i->first].append(i->second);
     }

src/os/bluestore/BlueStore.cc

@@ -12570,6 +12570,7 @@ int BlueStore::getattrs(
       r = -ENOENT;
       goto out;
     }
+    aset.clear();
     for (auto& i : o->onode.attrs) {
       aset.emplace(i.first.c_str(), i.second);
     }

src/osd/PGBackend.cc

@@ -615,42 +615,61 @@ int PGBackend::be_scan_list(
   ceph_assert(pos.pos < pos.ls.size());
   hobject_t& poid = pos.ls[pos.pos];
 
-  struct stat st;
-  int r = store->stat(
-    ch,
-    ghobject_t(
-      poid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard),
-    &st,
-    true);
-  if (r == 0) {
-    ScrubMap::object &o = map.objects[poid];
-    o.size = st.st_size;
-    ceph_assert(!o.negative);
-    store->getattrs(
+  int r = 0;
+  ScrubMap::object &o = map.objects[poid];
+  if (!pos.metadata_done) {
+    struct stat st;
+    r = store->stat(
       ch,
       ghobject_t(
 	poid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard),
-      o.attrs);
+      &st,
+      true);
+
+    if (r == 0) {
+      o.size = st.st_size;
+      ceph_assert(!o.negative);
+      r = store->getattrs(
+	ch,
+	ghobject_t(
+	  poid, ghobject_t::NO_GEN, get_parent()->whoami_shard().shard),
+	o.attrs);
+    }
 
-    if (pos.deep) {
-      r = be_deep_scrub(poid, map, pos, o);
+    if (r == -ENOENT) {
+      dout(25) << __func__ << "  " << poid << " got " << r
+	       << ", removing from map" << dendl;
+      map.objects.erase(poid);
+    } else if (r == -EIO) {
+      dout(25) << __func__ << "  " << poid << " got " << r
+	       << ", stat_error" << dendl;
+      o.stat_error = true;
+    } else if (r != 0) {
+      derr << __func__ << " got: " << cpp_strerror(r) << dendl;
+      ceph_abort();
+    }
+
+    if (r != 0) {
+      dout(25) << __func__ << "  " << poid << " got " << r
+	       << ", skipping" << dendl;
+      pos.next_object();
+      return 0;
     }
+
     dout(25) << __func__ << "  " << poid << dendl;
-  } else if (r == -ENOENT) {
-    dout(25) << __func__ << "  " << poid << " got " << r
-	     << ", skipping" << dendl;
-  } else if (r == -EIO) {
-    dout(25) << __func__ << "  " << poid << " got " << r
-	     << ", stat_error" << dendl;
-    ScrubMap::object &o = map.objects[poid];
-    o.stat_error = true;
-  } else {
-    derr << __func__ << " got: " << cpp_strerror(r) << dendl;
-    ceph_abort();
+    pos.metadata_done = true;
   }
-  if (r == -EINPROGRESS) {
-    return -EINPROGRESS;
+
+  if (pos.deep) {
+    r = be_deep_scrub(poid, map, pos, o);
+    if (r == -EINPROGRESS) {
+      return -EINPROGRESS;
+    } else if (r != 0) {
+      derr << __func__ << " be_deep_scrub got: " << cpp_strerror(r) << dendl;
+      ceph_abort();
+    }
   }
+
   pos.next_object();
   return 0;
 }

src/osd/osd_types.h

@@ -6302,6 +6302,7 @@ WRITE_CLASS_ENCODER(ScrubMap)
 struct ScrubMapBuilder {
   bool deep = false;
   std::vector<hobject_t> ls;
+  bool metadata_done = false;
   size_t pos = 0;
   int64_t data_pos = 0;
   std::string omap_pos;
@@ -6326,6 +6327,7 @@ struct ScrubMapBuilder {
 
   void next_object() {
     ++pos;
+    metadata_done = false;
     data_pos = 0;
     omap_pos.clear();
     omap_keys = 0;
@@ -6337,6 +6339,7 @@ struct ScrubMapBuilder {
     if (pos.pos < pos.ls.size()) {
       out << " " << pos.ls[pos.pos];
     }
+    out << " metadata_done " << pos.metadata_done;
     if (pos.data_pos < 0) {
       out << " byte " << pos.data_pos;
     }