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: v12.2.6~61^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c1ffd63692024f810f22e99fad3ae53de12b259e;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 (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 e0734ecd68a..0452b0f6a46 100644 --- 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> missing_digest; + map, + boost::optional>> missing_digest; map maps; maps[pg_whoami] = &scrubber.primary_scrubmap; diff --git a/src/osd/PG.h b/src/osd/PG.h index e4b1012345f..932dc51a181 100644 --- 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> &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 8b10e1f53ce..97cf4faf5cc 100644 --- a/src/osd/PGBackend.cc +++ b/src/osd/PGBackend.cc @@ -934,7 +934,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, @@ -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 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 eb80b7ce294..d69e511d36f 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -584,7 +584,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 8637c63d405..e92c6d24654 100644 --- 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 &head, */ void PrimaryLogPG::scrub_snapshot_metadata( ScrubMap &scrubmap, - const map> &missing_digest) + const map, + boost::optional>> &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>::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 daeac1e6a6e..3f10cef1875 100644 --- 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> &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;