From: xie xingguo Date: Sat, 30 Sep 2017 08:49:20 +0000 (+0800) Subject: osd/PrimaryLogPG: do not set data/omap digest blindly X-Git-Tag: v13.0.1~686^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F18061%2Fhead;p=ceph.git 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 --- diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 432d536b6212..52a8fbd3f35e 100644 --- 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> missing_digest; + map, + boost::optional>> 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 d9eb6a3fd074..5c7f4de0bd9c 100644 --- 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> &missing_digest) { } + const std::map, + boost::optional>> &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 eb1e0055f39d..04121f4b88ec 100644 --- a/src/osd/PGBackend.cc +++ b/src/osd/PGBackend.cc @@ -908,7 +908,8 @@ void PGBackend::be_compare_scrubmaps( map> &missing, map> &inconsistent, map> &authoritative, - map> &missing_digest, + map, + boost::optional>> &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 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 f244f4c2f703..33730fb48844 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -575,7 +575,8 @@ typedef ceph::shared_ptr OSDMapRef; map> &missing, map> &inconsistent, map> &authoritative, - map> &missing_digest, + map, + boost::optional>> &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 d89eef9bad76..a8d935f92557 100644 --- 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 &head, */ void PrimaryLogPG::scrub_snapshot_metadata( ScrubMap &scrubmap, - const map> &missing_digest) + const map, + boost::optional>> &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>::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 3e9de77ef767..eabd3c71204b 100644 --- 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> &missing_digest) override; + const std::map, + boost::optional>> &missing_digest) override; void _scrub_clear_state() override; void _scrub_finish() override; object_stat_collection_t scrub_cstat;