osd/PrimaryLogPG: do not set data/omap digest blindly

As bluestore has bulitin csum, we generally no longer generate
object data digest for now. The consequence is that we should
handle data/omap digest more carefully to make certain ops,
such as copy_from/promote, to work properly since they heavily
relies on data digest for data transfer correctness.

Example of failure:
http://pulpito.ceph.com/xxg-2017-09-30_11:46:34-rbd-master-distro-basic-mira/1690609/

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
(cherry picked from commit be078c8b7b131764caa28bc44452b8c5c2339623)

Conflicts:
	src/osd/PrimaryLogPG.cc (still have snapdir)

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index e0734ecd68a9d8e2967228b6b37d58944f27c55c..0452b0f6a46e51d7252d2b9b077d4a1491c5d6cf 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -5017,7 +5017,9 @@ void PG::scrub_compare_maps()
 
   // construct authoritative scrub map for type specific scrubbing
   scrubber.cleaned_meta_map.insert(scrubber.primary_scrubmap);
-  map<hobject_t, pair<uint32_t, uint32_t>> missing_digest;
+  map<hobject_t,
+      pair<boost::optional<uint32_t>,
+           boost::optional<uint32_t>>> missing_digest;
 
   map<pg_shard_t, ScrubMap *> maps;
   maps[pg_whoami] = &scrubber.primary_scrubmap;

diff --git a/src/osd/PG.h b/src/osd/PG.h
index e4b1012345f6099dcb3cd9c45e1b6d359dc18ec7..932dc51a181c3bd00567c5fc882bac5aee031d55 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -1431,7 +1431,9 @@ public:
     const hobject_t &begin, const hobject_t &end) = 0;
   virtual void scrub_snapshot_metadata(
     ScrubMap &map,
-    const std::map<hobject_t, pair<uint32_t, uint32_t>> &missing_digest) { }
+    const std::map<hobject_t,
+                   pair<boost::optional<uint32_t>,
+                        boost::optional<uint32_t>>> &missing_digest) { }
   virtual void _scrub_clear_state() { }
   virtual void _scrub_finish() { }
   virtual void split_colls(

diff --git a/src/osd/PGBackend.cc b/src/osd/PGBackend.cc
index 8b10e1f53cedeb741d0b7e40bb6cc350dc4f8c7d..97cf4faf5cc27f2fd0bba40b608f69c0a4855344 100644 (file)
--- a/src/osd/PGBackend.cc
+++ b/src/osd/PGBackend.cc
@@ -934,7 +934,8 @@ void PGBackend::be_compare_scrubmaps(
   map<hobject_t, set<pg_shard_t>> &missing,
   map<hobject_t, set<pg_shard_t>> &inconsistent,
   map<hobject_t, list<pg_shard_t>> &authoritative,
-  map<hobject_t, pair<uint32_t,uint32_t>> &missing_digest,
+  map<hobject_t, pair<boost::optional<uint32_t>,
+                      boost::optional<uint32_t>>> &missing_digest,
   int &shallow_errors, int &deep_errors,
   Scrub::Store *store,
   const spg_t& pgid,
@@ -1098,8 +1099,14 @@ void PGBackend::be_compare_scrubmaps(
        if (update == FORCE ||
            age > cct->_conf->osd_deep_scrub_update_digest_min_age) {
          dout(20) << __func__ << " will update digest on " << *k << dendl;
-         missing_digest[*k] = make_pair(auth_object.digest,
-                                        auth_object.omap_digest);
+          boost::optional<uint32_t> data_digest, omap_digest;
+          if (auth_oi.is_data_digest()) {
+            data_digest = auth_object.digest;
+          }
+          if (auth_oi.is_omap_digest()) {
+            omap_digest = auth_object.omap_digest;
+          }
+         missing_digest[*k] = make_pair(data_digest, omap_digest);
        } else {
          dout(20) << __func__ << " missing digest but age " << age
                   << " < " << cct->_conf->osd_deep_scrub_update_digest_min_age

diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h
index eb80b7ce294ff70e75fab394a9021dbeaaa8c6bb..d69e511d36f90a7a1ff68acac9fdab539044262c 100644 (file)
--- a/src/osd/PGBackend.h
+++ b/src/osd/PGBackend.h
@@ -584,7 +584,8 @@ typedef ceph::shared_ptr<const OSDMap> OSDMapRef;
      map<hobject_t, set<pg_shard_t>> &missing,
      map<hobject_t, set<pg_shard_t>> &inconsistent,
      map<hobject_t, list<pg_shard_t>> &authoritative,
-     map<hobject_t, pair<uint32_t,uint32_t>> &missing_digest,
+     map<hobject_t, pair<boost::optional<uint32_t>,
+                         boost::optional<uint32_t>>> &missing_digest,
      int &shallow_errors, int &deep_errors,
      Scrub::Store *store,
      const spg_t& pgid,

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index 8637c63d405cea9dd0ace24e67464affb49697e0..e92c6d2465412228d5a6e8af4fb5d75ec0d4d570 100644 (file)
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -8438,8 +8438,16 @@ void PrimaryLogPG::finish_copyfrom(CopyFromCallback *cb)
   // CopyFromCallback fills this in for us
   obs.oi.user_version = ctx->user_at_version;
 
-  obs.oi.set_data_digest(cb->results->data_digest);
-  obs.oi.set_omap_digest(cb->results->omap_digest);
+  if (cb->results->is_data_digest()) {
+    obs.oi.set_data_digest(cb->results->data_digest);
+  } else {
+    obs.oi.clear_data_digest();
+  }
+  if (cb->results->is_omap_digest()) {
+    obs.oi.set_omap_digest(cb->results->omap_digest);
+  } else {
+    obs.oi.clear_omap_digest();
+  }
 
   obs.oi.truncate_seq = cb->results->truncate_seq;
   obs.oi.truncate_size = cb->results->truncate_size;
@@ -8630,11 +8638,16 @@ void PrimaryLogPG::finish_promote(int r, CopyResults *results,
     }
     tctx->new_obs.oi.size = results->object_size;
     tctx->new_obs.oi.user_version = results->user_version;
-    // Don't care src object whether have data or omap digest
-    if (results->object_size)
+    if (results->is_data_digest()) {
       tctx->new_obs.oi.set_data_digest(results->data_digest);
-    if (results->has_omap)
+    } else {
+      tctx->new_obs.oi.clear_data_digest();
+    }
+    if (results->is_omap_digest()) {
       tctx->new_obs.oi.set_omap_digest(results->omap_digest);
+    } else {
+      tctx->new_obs.oi.clear_omap_digest();
+    }
     tctx->new_obs.oi.truncate_seq = results->truncate_seq;
     tctx->new_obs.oi.truncate_size = results->truncate_size;
 
@@ -13827,7 +13840,9 @@ unsigned PrimaryLogPG::process_clones_to(const boost::optional<hobject_t> &head,
  */
 void PrimaryLogPG::scrub_snapshot_metadata(
   ScrubMap &scrubmap,
-  const map<hobject_t, pair<uint32_t, uint32_t>> &missing_digest)
+  const map<hobject_t,
+            pair<boost::optional<uint32_t>,
+                 boost::optional<uint32_t>>> &missing_digest)
 {
   dout(10) << __func__ << dendl;
 
@@ -14169,10 +14184,7 @@ void PrimaryLogPG::scrub_snapshot_metadata(
   if (head && (head_error.errors || soid_error_count))
     scrubber.store->add_snap_error(pool.id, head_error);
 
-  for (map<hobject_t,pair<uint32_t,uint32_t>>::const_iterator p =
-        missing_digest.begin();
-       p != missing_digest.end();
-       ++p) {
+  for (auto p = missing_digest.begin(); p != missing_digest.end(); ++p) {
     if (p->first.is_snapdir())
       continue;
     dout(10) << __func__ << " recording digests for " << p->first << dendl;
@@ -14192,8 +14204,16 @@ void PrimaryLogPG::scrub_snapshot_metadata(
     OpContextUPtr ctx = simple_opc_create(obc);
     ctx->at_version = get_next_version();
     ctx->mtime = utime_t();      // do not update mtime
-    ctx->new_obs.oi.set_data_digest(p->second.first);
-    ctx->new_obs.oi.set_omap_digest(p->second.second);
+    if (p->second.first) {
+      ctx->new_obs.oi.set_data_digest(*p->second.first);
+    } else {
+      ctx->new_obs.oi.clear_data_digest();
+    }
+    if (p->second.second) {
+      ctx->new_obs.oi.set_omap_digest(*p->second.second);
+    } else {
+      ctx->new_obs.oi.clear_omap_digest();
+    }
     finish_ctx(ctx.get(), pg_log_entry_t::MODIFY);
 
     ctx->register_on_success(

diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h
index daeac1e6a6eaf08d28fa1cef36543f774b15c6c9..3f10cef187526c3ad2c0457989a6daf8f12a82fa 100644 (file)
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -1321,7 +1321,9 @@ protected:
     const hobject_t &begin, const hobject_t &end) override;
   void scrub_snapshot_metadata(
     ScrubMap &map,
-    const std::map<hobject_t, pair<uint32_t, uint32_t>> &missing_digest) override;
+    const std::map<hobject_t,
+                   pair<boost::optional<uint32_t>,
+                        boost::optional<uint32_t>>> &missing_digest) override;
   void _scrub_clear_state() override;
   void _scrub_finish() override;
   object_stat_collection_t scrub_cstat;