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>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 432d536b6212245697ecf59e94964a11077d7728..52a8fbd3f35eb77be9d21fad34847115fb872b16 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -4558,7 +4558,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;
 
   if (acting.size() > 1) {
     dout(10) << __func__ << "  comparing replica scrub maps" << dendl;

diff --git a/src/osd/PG.h b/src/osd/PG.h
index d9eb6a3fd074cbbb4a309e528018faa16d5c8670..5c7f4de0bd9c60d866e6cf5355bfb2776f8b1b59 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -1377,7 +1377,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 eb1e0055f39d9e9204f2432f89f990c0a300bf1b..04121f4b88ec246cdd141317b20ad0a8918f067b 100644 (file)
--- a/src/osd/PGBackend.cc
+++ b/src/osd/PGBackend.cc
@@ -908,7 +908,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,
@@ -1081,8 +1082,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 f244f4c2f703dcfc7cbf96e73166b40a2c2c60cd..33730fb488441a9b3a95269bec557ff4d60ca78c 100644 (file)
--- a/src/osd/PGBackend.h
+++ b/src/osd/PGBackend.h
@@ -575,7 +575,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 d89eef9bad763076d0e13ee9a23ef87f39ab6043..a8d935f92557fa5725e149da75e117c7e4490a16 100644 (file)
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -8094,8 +8094,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;
@@ -8283,11 +8291,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;
 
@@ -13224,7 +13237,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;
 
@@ -13499,10 +13514,7 @@ void PrimaryLogPG::scrub_snapshot_metadata(
   if (head && head_error.errors)
     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) {
     assert(!p->first.is_snapdir());
     dout(10) << __func__ << " recording digests for " << p->first << dendl;
     ObjectContextRef obc = get_object_context(p->first, false);
@@ -13521,8 +13533,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 3e9de77ef7671cd2fd7c7f913298354ec044f4fb..eabd3c71204b4adb74968e7dd72d06c8a2fa34ad 100644 (file)
--- a/src/osd/PrimaryLogPG.h
+++ b/src/osd/PrimaryLogPG.h
@@ -1308,7 +1308,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;