From: Neha Ojha Date: Wed, 6 Feb 2019 03:23:21 +0000 (-0800) Subject: osd/PGLog: should not rollback further than deleted object version X-Git-Tag: v13.2.6~63^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a42ca17454198a13d340a1d8a22d012b2b6f000f;p=ceph.git osd/PGLog: should not rollback further than deleted object version When a deleted object becomes a divergent entry in the pg log, we should not be able to rollback to a version of the deleted object that doesn't exist. To avoid this, we need to preserve the original crt of the pg log, before we update it in rewind_from_head() and use that to decide whether we can rollback or not in _merge_object_divergent_entries(). Fixes: http://tracker.ceph.com/issues/36739 Signed-off-by: Neha Ojha (cherry picked from commit de18c592259816ad013ac82983cdec71bbfa51a0) Conflicts: src/osd/PGLog.h : Resolved in struct PGLog --- diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc index eb88dc1db3c5..278c1720e337 100644 --- a/src/osd/PGLog.cc +++ b/src/osd/PGLog.cc @@ -262,6 +262,7 @@ void PGLog::proc_replica_log( divergent, oinfo, olog.get_can_rollback_to(), + olog.get_can_rollback_to(), omissing, 0, this); @@ -304,7 +305,11 @@ void PGLog::rewind_divergent_log(eversion_t newhead, dout(10) << "rewind_divergent_log truncate divergent future " << newhead << dendl; - + // We need to preserve the original crt before it gets updated in rewind_from_head(). + // Later, in merge_object_divergent_entries(), we use it to check whether we can rollback + // a divergent entry or not. + eversion_t original_crt = log.get_can_rollback_to(); + dout(20) << __func__ << " original_crt = " << original_crt << dendl; if (info.last_complete > newhead) info.last_complete = newhead; @@ -322,6 +327,7 @@ void PGLog::rewind_divergent_log(eversion_t newhead, divergent, info, log.get_can_rollback_to(), + original_crt, missing, rollbacker, this); @@ -443,6 +449,7 @@ void PGLog::merge_log(pg_info_t &oinfo, pg_log_t &olog, pg_shard_t fromosd, divergent, info, log.get_can_rollback_to(), + log.get_can_rollback_to(), missing, rollbacker, this); diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h index 6cf3cd99d0c1..d32007460962 100644 --- a/src/osd/PGLog.h +++ b/src/osd/PGLog.h @@ -843,6 +843,7 @@ protected: const mempool::osd_pglog::list &orig_entries, ///< [in] entries for hoid to merge const pg_info_t &info, ///< [in] info for merging entries eversion_t olog_can_rollback_to, ///< [in] rollback boundary + eversion_t original_can_rollback_to, ///< [in] original rollback boundary missing_type &missing, ///< [in,out] missing to adjust, use LogEntryHandler *rollbacker, ///< [in] optional rollbacker object const DoutPrefixProvider *dpp ///< [in] logging provider @@ -906,6 +907,8 @@ protected: const bool object_not_in_store = !missing.is_missing(hoid) && entries.rbegin()->is_delete(); + ldpp_dout(dpp, 10) << __func__ << ": hoid " << " object_not_in_store: " + << object_not_in_store << dendl; ldpp_dout(dpp, 10) << __func__ << ": hoid " << hoid << " prior_version: " << prior_version << " first_divergent_update: " << first_divergent_update @@ -1008,7 +1011,10 @@ protected: for (list::const_reverse_iterator i = entries.rbegin(); i != entries.rend(); ++i) { - if (!i->can_rollback() || i->version <= olog_can_rollback_to) { + /// Use original_can_rollback_to instead of olog_can_rollback_to to check + // if we can rollback or not. This is to ensure that we don't try to rollback + // to an object that has been deleted and doesn't exist. + if (!i->can_rollback() || i->version <= original_can_rollback_to) { ldpp_dout(dpp, 10) << __func__ << ": hoid " << hoid << " cannot rollback " << *i << dendl; can_rollback = false; @@ -1021,7 +1027,7 @@ protected: for (list::const_reverse_iterator i = entries.rbegin(); i != entries.rend(); ++i) { - assert(i->can_rollback() && i->version > olog_can_rollback_to); + assert(i->can_rollback() && i->version > original_can_rollback_to); ldpp_dout(dpp, 10) << __func__ << ": hoid " << hoid << " rolling back " << *i << dendl; if (rollbacker) @@ -1058,6 +1064,7 @@ protected: mempool::osd_pglog::list &entries, ///< [in] entries to merge const pg_info_t &oinfo, ///< [in] info for merging entries eversion_t olog_can_rollback_to, ///< [in] rollback boundary + eversion_t original_can_rollback_to, ///< [in] original rollback boundary missing_type &omissing, ///< [in,out] missing to adjust, use LogEntryHandler *rollbacker, ///< [in] optional rollbacker object const DoutPrefixProvider *dpp ///< [in] logging provider @@ -1073,6 +1080,7 @@ protected: i->second, oinfo, olog_can_rollback_to, + original_can_rollback_to, omissing, rollbacker, dpp); @@ -1096,6 +1104,7 @@ protected: entries, info, log.get_can_rollback_to(), + log.get_can_rollback_to(), missing, rollbacker, this); diff --git a/src/test/osd/TestPGLog.cc b/src/test/osd/TestPGLog.cc index f1def5c2df54..3eda4d18f721 100644 --- a/src/test/osd/TestPGLog.cc +++ b/src/test/osd/TestPGLog.cc @@ -2939,6 +2939,7 @@ TEST_F(PGLogTest, _merge_object_divergent_entries) { _merge_object_divergent_entries(log, hoid, orig_entries, oinfo, log.get_can_rollback_to(), + log.get_can_rollback_to(), missing, &rollbacker, this); // No core dump @@ -2965,6 +2966,7 @@ TEST_F(PGLogTest, _merge_object_divergent_entries) { _merge_object_divergent_entries(log, hoid, orig_entries, oinfo, log.get_can_rollback_to(), + log.get_can_rollback_to(), missing, &rollbacker, this); // No core dump