]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
osd: Optimized EC apply_pwlc needs to be careful about advancing last_complete
authorBill Scales <bill_scales@uk.ibm.com>
Wed, 16 Jul 2025 13:55:49 +0000 (14:55 +0100)
committerAlex Ainscow <aainscow@uk.ibm.com>
Sun, 7 Sep 2025 23:10:41 +0000 (00:10 +0100)
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)

src/osd/PeeringState.cc

index ce520a9f4b5dd9df493398de9fa5067343aa6ac1..3b9a7c195ced18f936f045e05c6dd22d73ab3975 100644 (file)
@@ -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;
   }
 }