Skip to content

Commit

Permalink
osd/PG: simplify start_recovery_ops return value meaning
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
liewegas committed Oct 4, 2017
1 parent edb4965 commit ea32e82
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
4 changes: 2 additions & 2 deletions src/osd/OSD.cc
Expand Up @@ -8837,11 +8837,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);
Expand Down
10 changes: 5 additions & 5 deletions src/osd/PrimaryLogPG.cc
Expand Up @@ -11079,7 +11079,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();
Expand Down Expand Up @@ -11144,7 +11144,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);
Expand All @@ -11158,22 +11158,22 @@ 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()) {
// this shouldn't happen!
// 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)) {
Expand Down

0 comments on commit ea32e82

Please sign in to comment.