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: Limit pg log length during recovery/backfill so that we don't run out of memory #23211

Merged
merged 11 commits into from Sep 12, 2018

Conversation

Projects
None yet
6 participants

neha-ojha added some commits Jul 16, 2018

osd: make calc_trim_to() independent of min_last_complete_ondisk
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit 1ae5fd3)

Conflicts:
	src/osd/PrimaryLogPG.cc: min_last_complete_ondisk and
        pg_log.get_can_rollback_to() are no longer the limit of the pg log.
        Make the head of the pg log the new limit for pg log trimming.
osd: print pg log length and trim_to
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit f48584a)
osd: handle trim() during backfill
Remove async recovery components: The async recovery feature is not present
in luminous. We do not need commit 22d17fb,
which adds a flag to remember async recovery. We have also removed async
recovery requirements from this commit and modified the commit message to
only reflect backfill.

Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit e538c31)
osd: allow trim() to proceed when there are missing items
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit de42fee)

Conflicts:
	src/osd/PGLog.cc: The async recovery feature is not present
        in luminous. Remove async recovery requirements from this commit.
osd: reset complete_to when trimming the log past it
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit 38170cd)
osd/PGLog: allow pg log trim when complete_to is less than trim_to
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit a5329ba)

Conflicts:
	src/osd/PGLog.cc: Now it is possible to have complete_to version
        less than or equal to trim version, because the pg log length upper
        limit is a hard limit, and trim can proceed even when there is
        pending recovery/backfill. So do not complain when this happens.

@neha-ojha neha-ojha added the core label Jul 24, 2018

@neha-ojha neha-ojha requested a review from jdurgin Jul 24, 2018

@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Jul 25, 2018

retest this please

@neha-ojha neha-ojha added the needs-qa label Jul 25, 2018

@neha-ojha neha-ojha changed the title from luminous: osd: Limit pg log length during recovery/backfill so that we don't run out of memory to [DNM]luminous: osd: Limit pg log length during recovery/backfill so that we don't run out of memory Jul 30, 2018

neha-ojha added some commits Jul 30, 2018

osd/PGLog.cc: use lgeneric_subdout instead of generic_dout
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit 1b6dafb)
src/osd/PG.cc: remove redundant call to trim_log()
This change is motived by the failure tracked in
https://tracker.ceph.com/issues/25198. The failure highlights a case, when a
call to trim_log() after the PG has recovered, races with the previous op,
on a replica OSD. Since the previous operation has not completed, the
last_complete value for that OSD is not valid, when we try to trim the
log. It is also worth noting that the race is due to MOSDPGTrim going through
the strict queue as a peering message vs regular ops going through the
non-strict queue.

During the investigation of this bug, we noticed that, with
https://tracker.ceph.com/issues/23979, we allow pg log trimming to
happen on the primary and replicas, whenever we cross the upper bound of
the pg log. This also ensures that pg log trimming happens while processing
any new op.

Therefore, the function trim_log(), which earlier served the purpose of
trimming logs on the primary and replicas, just before the PG went into
the Recovered state, is no more required. This acted like a last line of
defense to trim logs, when we did not need the logs any more. But, this call
seems redundant now, because, we are limiting the pg log length at all times.

Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit 283b0bd)

Conflicts:
	src/osd/PG.cc: We do not need the trim_log() call any longer,
        due to the explanation provided in the commit message.
@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Aug 2, 2018

@yuriw

This comment has been minimized.

Contributor

yuriw commented Aug 2, 2018

@yuriw yuriw changed the title from [DNM]luminous: osd: Limit pg log length during recovery/backfill so that we don't run out of memory to luminous: osd: Limit pg log length during recovery/backfill so that we don't run out of memory Aug 2, 2018

@yuriw yuriw added the DNM label Aug 2, 2018

@liewegas liewegas added this to the luminous milestone Aug 3, 2018

@yuriw

This comment has been minimized.

Contributor

yuriw commented Aug 6, 2018

@neha-ojha pls add tags when its ready for retesting

osd/PGLog.cc: check if complete_to points to log.end()
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit 630daa1)
@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Aug 7, 2018

updated to incorporate https://tracker.ceph.com/issues/26868.
TODO: add backport tracker ticket reference

@yuriw

This comment has been minimized.

Contributor

yuriw commented Aug 10, 2018

@jdurgin it passed tests, pls merge

@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Aug 10, 2018

https://tracker.ceph.com/issues/21416 appears in the multimds suite. Let's try to figure this out before merging this.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Aug 20, 2018

@liewegas @jdurgin Should this be considered a blocker for 12.2.8?

@smithfarm smithfarm removed the 12.2.8 label Aug 20, 2018

@jdurgin

This comment has been minimized.

Member

jdurgin commented Aug 20, 2018

@smithfarm not a blocker for 12.2.8, since we are still finding bugs let's hold off on this one until 12.2.9

osd/PrimaryLogPG.cc: limit trimming at can_rollback_to
This change is motivated by the failures seen in the multimds suite,
where we hit assert(s <= can_rollback_to), while trimming the log in ec
pools.

This is due to the fact that we had removed limits on the trim_to value to
address https://tracker.ceph.com/issues/23979.

But, seems that this could be dangerous for ec pools. So, keep the
can_rollback_to limit, while calculating the trim_to value.

Fixes: http://tracker.ceph.com/issues/21416
Signed-off-by: Neha Ojha <nojha@redhat.com>
(cherry picked from commit 4b5c6b8)
@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Aug 30, 2018

@yuriw This is ready for retesting with the rados and multimds suite.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Sep 4, 2018

@neha-ojha Does this need #23894 as well? (not yet merged into master)

@smithfarm smithfarm added the feature label Sep 4, 2018

@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Sep 7, 2018

@smithfarm yes, I can backport #23894 to this PR, when it gets merged.

@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Sep 7, 2018

retest this please

osd/PrimaryLogPG: avoid dereferencing invalid complete_to
For the auto-repair (EIO caused) case, we will not reinitialize
**complete_to** (because last_complete is equal to last_update!)
and hence there is chance that **complete_to** should aleady
point to **log.end()** before we call recover_got.

We could simply drop it here as we (already) logged the **complete_to**
iterator change in a more compatible way a few lines below.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
(cherry picked from commit 69a2cc3)
@neha-ojha

This comment has been minimized.

Member

neha-ojha commented Sep 7, 2018

backported #23894, @yuriw is this ready for testing with the rados and multimds suite.

@yuriw

This comment has been minimized.

Contributor

yuriw commented Sep 7, 2018

@smithfarm this is ready for 12.2.9, pls tag (completely up to you if you want to use 12.2.9 tag or the old way)

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Sep 7, 2018

jenkins test docs

@smithfarm smithfarm added luminous-batch-1 and removed DNM labels Sep 7, 2018

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Sep 7, 2018

@yuriw There's no 12.2.9 taglabel, so I used the old way :-)

@yuriw

This comment has been minimized.

Contributor

yuriw commented Sep 10, 2018

@yuriw yuriw merged commit 2e321e1 into ceph:luminous Sep 12, 2018

4 checks passed

Docs: build check OK - docs built
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment