osd: PGLog Attach correct version to missing list when ignoring log entries

A previous fix for PR 66698 fixed an issue where log entries associated with
partial writes were being processed incorrectly (see that PR and associated
tracker for details).  The fix was to ignore log entries that should not have
been present on the non-primary shard.

The problem with that approach is that in a more complex scenario, where the
log contained a partial write, followed by a full write AND the shard is
backfilling, then the missing list was being given the version prior to the
full write, rather than prior to the clone.

Our fix here corrects how the missing list version is calculated.

See the associated tracker for instructions on how to recreate

Conflicts:
	a/src/test/osd/TestECFailoverWithPeering.cc

Unittest not backported because it is based on a new framework in Umbrella

Fixes: https://tracker.ceph.com/issues/75211
Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>
(cherry picked from commit 7967f28c774f3f3830f10e02e88d6ef3e82b9f82)

diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc
index a5f3b1b9541c04270c0898e6eb967c952e4f9d67..8007ccfec71721d96ad4628c49084296a3b93731 100644 (file)
--- a/src/osd/PGLog.cc
+++ b/src/osd/PGLog.cc
@@ -304,7 +304,7 @@ void PGLog::proc_replica_log(
     omissing,
     nullptr,
     ec_optimizations_enabled,
-    to.shard,
+    from.shard,
     this);
 
   if (lu < oinfo.last_update) {

diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h
index 1f9f899fc0ba1577f1123d533eca4c7e9ad31439..43d4518030aa2552345f61a4fb3a4dd7a2741068 100644 (file)
--- a/src/osd/PGLog.h
+++ b/src/osd/PGLog.h
@@ -1115,6 +1115,7 @@ protected:
     mempool::osd_pglog::list<pg_log_entry_t> entries;
     eversion_t last;
     bool seen_non_error = false;
+    std::optional<eversion_t> prior_version_opt;
     for (auto i = orig_entries.begin();
         i != orig_entries.end();
         ++i) {
@@ -1147,14 +1148,20 @@ protected:
        }
       }
       if (i->is_error()) {
- ldpp_dout(dpp, 20) << __func__ << ": ignoring " << *i << dendl;
+        ldpp_dout(dpp, 20) << __func__ << ": ignoring " << *i << dendl;
       } else if (!i->written_shards.empty() && !i->written_shards.contains(orig_shard)) {
         ldpp_dout(dpp, 20) << __func__ << ": ignoring partial write " << *i << dendl;
- last = i->version;
+        last = i->version;
+        if (!prior_version_opt) {
+          prior_version_opt = i->prior_version;
+        }
       } else {
- ldpp_dout(dpp, 20) << __func__ << ": keeping " << *i << dendl;
- entries.push_back(*i);
- last = i->version;
+        ldpp_dout(dpp, 20) << __func__ << ": keeping " << *i << dendl;
+        if (!prior_version_opt) {
+          prior_version_opt = i->prior_version;
+        }
+        entries.push_back(*i);
+        last = i->version;
       }
     }
     if (entries.empty()) {
@@ -1162,7 +1169,8 @@ protected:
       return;
     }
 
-    const eversion_t prior_version = entries.begin()->prior_version;
+    ceph_assert(prior_version_opt);
+    const eversion_t prior_version = *prior_version_opt;
     const eversion_t first_divergent_update = entries.begin()->version;
     const eversion_t last_divergent_update = entries.rbegin()->version;
     const bool object_not_in_store =