From: Bill Scales <bill_scales@uk.ibm.com>
Date: Wed, 16 Jul 2025 13:55:49 +0000 (+0100)
Subject: osd: Optimized EC apply_pwlc needs to be careful about advancing last_complete
X-Git-Tag: testing/wip-jcollin-testing-20250917.124222-tentacle~1^2~2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=4c26de4003664581057720a22a9faacbe7324707;p=ceph-ci.git

osd: Optimized EC apply_pwlc needs to be careful about advancing last_complete

Fix bug in apply_pwlc where the primary was advancing last_complete for a
shard doing async recorvery so that last_complete became equal to last_update
and it then thought that recovery had completed. It is only valid to advance
last_complete if it is equal to last_update.

Tidy up the logging in this function as consecutive calls to this function
often logged that it could advance on the 1st call and then that it could not
on the 2nd call. We only want one log message.

Signed-off-by: Bill Scales <bill_scales@uk.ibm.com>
(cherry picked from commit 0b19ed49ff76d7470bfcbd7f26ea0c7e5a2bc358)
---

diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index ce520a9f4b5..3b9a7c195ce 100644
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -330,42 +330,36 @@ void PeeringState::apply_pwlc(const std::pair<eversion_t, eversion_t> pwlc,
   // Check if last_complete and last_update can be advanced based on
   // knowledge of partial_writes
   const auto & [fromversion, toversion] = pwlc;
-  bool logged = false;
-  if ((toversion > info.last_complete) &&
-      (fromversion.version <= info.last_complete.version)) {
-    psdout(10) << "osd." << shard << " has last_complete "
-	       << info.last_complete
-	       << " pwlc can advance last_complete to " << toversion
-	       << dendl;
-    logged = true;
-    info.last_complete = toversion;
-  }
-  if ((toversion > info.last_update) &&
-      (fromversion.version <= info.last_update.version)) {
-    if (!logged) {
+  if (toversion > info.last_update) {
+    if (fromversion.version <= info.last_update.version) {
+      if (info.last_complete == info.last_update) {
+	psdout(10) << "osd." << shard << " has last_complete"
+		   << "=last_update " << info.last_update
+		   << " pwlc can advance both to " << toversion
+		   << dendl;
+	info.last_complete = toversion;
+      } else {
+	psdout(10) << "osd." << shard << " has last_complete "
+		   << info.last_complete << " and last_update "
+		   << info.last_update
+		   << " pwlc can advance last_update to " << toversion
+		   << dendl;
+      }
+      info.last_update = toversion;
+      if (log1 && toversion > log1->head) {
+	log1->head = toversion;
+      }
+      if (log2 && toversion > log2->get_head()) {
+	log2->set_head(toversion);
+      }
+    } else {
       psdout(10) << "osd." << shard << " has last_complete "
 		 << info.last_complete << " and last_update "
 		 << info.last_update
-		 << " pwlc can advance last_update to " << toversion
+		 << " cannot apply pwlc from " << fromversion
+		 << " to " << toversion
 		 << dendl;
-      logged = true;
-    }
-    info.last_update = toversion;
-    if (log1 && toversion > log1->head) {
-      log1->head = toversion;
     }
-    if (log2 && toversion > log2->get_head()) {
-      log2->set_head(toversion);
-    }
-  }
-  if ((toversion > info.last_complete) &&
-      !logged) {
-    psdout(10) << "osd." << shard << " has last_complete "
-	       << info.last_complete << " and last_update "
-	       << info.last_update
-	       << " cannot apply pwlc from " << fromversion
-	       << " to " << toversion
-	       << dendl;
   }
 }