New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luminous: osd: allow recovery preemption #18025

Merged
merged 19 commits into from Oct 11, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Member

liewegas commented Sep 28, 2017

No description provided.

liewegas added some commits Sep 19, 2017

osd/PG: set {backfill,recovery}_wait when canceling backfill/recovery
The only caller currently is when we get as far as we can with backfill
or recovery but still have unfound objects.  In this case, go back into
the *_wait state instead of appearing as though we are still doing
something.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 4b21677)
osd/PG: make some trivial events TrivialEvent
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 3eadfa0)
osd/PG: specify delay in Cancel{Recovery,Backfill}
For now it is always the retry interval, but later perhaps not!

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 597dfd1)
osd/PG: Cancel{Recovery,Backfill} -> Defer{Recovery,Backfill}
"Defer" is more accurate here; we aren't canceling anything, just
rescheduling the work.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 2e45497)
osd/osd_types: remove weird BACKFILL state hack
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 5bcfaf4)
osd/osd_types: make BACKFILL <-> "backfilling" for parser
We render BACKFILL as "backfilling"; make sure parse works that
way too.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 6fa40e4)

liewegas added some commits Sep 19, 2017

common/AsyncReserver: support preemption
If an (optional) preemption context is provided, use that to preempt
and existing reservation and grant a higher-priority one.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit dbc002e)
osd/PG: allow local recovery reservations to be preempted
If a PG has a higher recovery priority and a lower-priority item is in
progress, allow it to be preempted.  This triggers the RecoveryCancel
or BackfillCancel event with a 0 delay, which means it will immediately
re-request a reservation (and presumably wait).

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit a8534cc)
osd: PG_STATE_BACKFILL -> PG_STATE_BACKFILLING
Match user-facing string

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 31a3494)

- add update to mon/PGMap.cc
osd/PG: handle racy preemption
If we finish recovery/backfill and go active, but also get
preempted at the same time, we can ignore the event.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit d8c3756)
common/AsyncReserver: get a cct
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 08d2c88)
qa/suites/rados/singleton/all/recovery-preemption: add test
This mirrors what I was testing locally.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit d7b29ac)
@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Oct 1, 2017

Member

do not backport until we fix http://tracker.ceph.com/issues/21613

Member

liewegas commented Oct 1, 2017

do not backport until we fix http://tracker.ceph.com/issues/21613

@liewegas liewegas added DNM and removed wip-sage-testing labels Oct 1, 2017

liewegas added some commits Oct 1, 2017

osd/PG: move reject_reservation out of RemoteReservationRejected reac…
…tion

The RemoteReservationRejected event is also submitted when we are a
replica or backfill target and get a MBackfillReserve REJECT message
because the primary canceled or was preempted.  In that case, we don't
want to send a REJECT back to the primary; we only need to send it in the
cases where *we*, locally, decide to reject.  Move the call to those call
sites.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 57d18f0)
osd/PG: cancel local reservation in RemoteReservationRejected handler
We can get a RemoteReservationRejected event either because *we* decide
to reject, or because we get a REJECT from the primary that means "cancel"
(e.g., because recovery/backfill was preempted there).  In both cases we
want to cancel our remote_reservation.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit f5809af)
osd/PG: ignore RemoteReservationRejected if we are RepNotRecoverying
The primary may send us a REJECT (meaning cancel) if recovery/backfill is
preempted there.  That can happen even if the recovery isn't reserved or
requested here (e.g., because the primary is still waiting for the local
reservation).  Just ignore it and remain in RepNotRecovering.

Fixes: http://tracker.ceph.com/issues/21613
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 1ce235c)
osd/PG: handle RecoveryReservationRejected in RepWaitRecoveryReserved
This state is analogous to RepWaitBackfillReserved; just like we do there
we want to handle the REJECT from the primary by canceling our local
remote_reservation.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit ab8f1d2)
osd: make note about when we get MBackfillReserve REJECT messages
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 6e829a3)
osd/PG: separate verb 'Reject' from passive 'rejected'
This reduces pg->reject_reservation() callsites from 2 to 1 and
makes the state transitions a bit more explicit.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit bf7f101)
osd/PG: separate event for RemoteReservationCanceled
Right now we transparently map a RemoteReservationRejected into a
*Canceled event because this what peers send over the wire.  Even
once new peers start sending and explicit CANCEL, old peers will
still do so, so we'll maintain this mapping for a while.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 84d71e6)

@liewegas liewegas added wip-sage2-testing and removed DNM labels Oct 5, 2017

@liewegas liewegas requested a review from gregsfortytwo Oct 6, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Oct 6, 2017

Member

@gregsfortytwo this is everything except the final patch that adds the CANCEL verb. The second to last one that adds the cancel TrivialEvent is not terribly useful but it helps clarify the code a bit and keeps us as close to master as possible.

Member

liewegas commented Oct 6, 2017

@gregsfortytwo this is everything except the final patch that adds the CANCEL verb. The second to last one that adds the cancel TrivialEvent is not terribly useful but it helps clarify the code a bit and keeps us as close to master as possible.

@gregsfortytwo

This comment has been minimized.

Show comment
Hide comment
@gregsfortytwo

gregsfortytwo Oct 6, 2017

Member

Is there any point to not backporting the final verb-changing patch? That'll keep us closer still to master ;)
Oh, it's conditioned on VERSION_MIMIC and giving it its own feature bit would be tedious...okay.

Anyway, I'm fine backporting these patches and I verified this PR has all the patches from #17839 and #18070; were there some backport conflicts you wanted me to check on it @liewegas?

Member

gregsfortytwo commented Oct 6, 2017

Is there any point to not backporting the final verb-changing patch? That'll keep us closer still to master ;)
Oh, it's conditioned on VERSION_MIMIC and giving it its own feature bit would be tedious...okay.

Anyway, I'm fine backporting these patches and I verified this PR has all the patches from #17839 and #18070; were there some backport conflicts you wanted me to check on it @liewegas?

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Oct 7, 2017

Member

The only conflict was from the PG_STATE_BACKFILL -> PG_STATE_BACKFILLING rename vs code in PGMap.cc that was removed in master, so I think we're good to go! Thanks!

Member

liewegas commented Oct 7, 2017

The only conflict was from the PG_STATE_BACKFILL -> PG_STATE_BACKFILLING rename vs code in PGMap.cc that was removed in master, so I think we're good to go! Thanks!

@liewegas liewegas merged commit 8ad4617 into ceph:luminous Oct 11, 2017

3 of 4 checks passed

Docs: build check Docs: failed with errors
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details

@liewegas liewegas deleted the liewegas:wip-recovery-preemption-luminous branch Oct 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment