osd/PG: simplify start_recovery_ops return value meaning

Instead of complex check in caller, return simple bool indicating whether
find_unfound() should be called with a rctx.

There is one remaining condition that can probably be simplified:

  if (!recovering.empty() ||
      work_in_progress || recovery_ops_active > 0 || deferred_backfill)
    return !work_in_progress && have_unfound();

but we will leave it for now.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 8d8c17c88482c23e5449a93d0c45374c75fa35c4..4a9b053c6d15a49505387695b4513112dcd9aaa4 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -8846,11 +8846,11 @@ void OSD::do_recovery(
     dout(20) << "  active was " << service.recovery_oids[pg->pg_id] << dendl;
 #endif
 
-    bool wip = pg->start_recovery_ops(reserved_pushes, handle, &started);
+    bool do_unfound = pg->start_recovery_ops(reserved_pushes, handle, &started);
     dout(10) << "do_recovery started " << started << "/" << reserved_pushes 
             << " on " << *pg << dendl;
 
-    if (!wip && pg->have_unfound()) {
+    if (do_unfound) {
       PG::RecoveryCtx rctx = create_context();
       rctx.handle = &handle;
       pg->find_unfound(queued, &rctx);

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index bcf07ed20b26203b8743767a9f4c864966ee1a6b..51ab07688f8f2456e45b141a5bd76db1e0d8f097 100644 (file)
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -11094,7 +11094,7 @@ bool PrimaryLogPG::start_recovery_ops(
     /* TODO: I think this case is broken and will make do_recovery()
      * unhappy since we're returning false */
     dout(10) << "recovery raced and were queued twice, ignoring!" << dendl;
-    return false;
+    return have_unfound();
   }
 
   const auto &missing = pg_log.get_missing();
@@ -11159,7 +11159,7 @@ bool PrimaryLogPG::start_recovery_ops(
 
   if (!recovering.empty() ||
       work_in_progress || recovery_ops_active > 0 || deferred_backfill)
-    return work_in_progress;
+    return !work_in_progress && have_unfound();
 
   assert(recovering.empty());
   assert(recovery_ops_active == 0);
@@ -11173,14 +11173,14 @@ bool PrimaryLogPG::start_recovery_ops(
   int unfound = get_num_unfound();
   if (unfound) {
     dout(10) << " still have " << unfound << " unfound" << dendl;
-    return work_in_progress;
+    return true;
   }
 
   if (missing.num_missing() > 0) {
     // this shouldn't happen!
     osd->clog->error() << info.pgid << " Unexpected Error: recovery ending with "
                       << missing.num_missing() << ": " << missing.get_items();
-    return work_in_progress;
+    return false;
   }
 
   if (needs_recovery()) {
@@ -11188,7 +11188,7 @@ bool PrimaryLogPG::start_recovery_ops(
     // We already checked num_missing() so we must have missing replicas
     osd->clog->error() << info.pgid 
                        << " Unexpected Error: recovery ending with missing replicas";
-    return work_in_progress;
+    return false;
   }
 
   if (state_test(PG_STATE_RECOVERING)) {