Skip to content
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

osd: use the sum of max allowed backfill and recovery as max_allowed #19036

Closed
wants to merge 1 commit into from

Conversation

tchaikov
Copy link
Contributor

…ax_allowed

also, it would be easier to trigger the preemption of recovery by
decreasing the reserver's max_allowed.

Fixes: http://tracker.ceph.com/issues/22043
Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov tchaikov changed the title osd: use the sum of max allowed backfill and recovery as reserver's m… osd: use the sum of max allowed backfill and recovery as max_allowed Nov 20, 2017
…ax_allowed

also, it would be easier to trigger the preemption of recovery by
decreasing the reserver's max_allowed.

Fixes: http://tracker.ceph.com/issues/22043
Signed-off-by: Kefu Chai <kchai@redhat.com>
@liewegas
Copy link
Member

I don't think this is right. The reserver max(es) really should be the # of PGs that are undergoing backfill, and the max_active is the number of ops in flight for those PGs.

@tchaikov
Copy link
Contributor Author

tchaikov commented Nov 21, 2017

I don't think this is right. The reserver max(es) really should be the # of PGs that are undergoing backfill,

the backfill and recovery are sharing the same local and remote reservers. so in addition to undergoing backfills, i think we should take the undergoing recoveries into account.

see https://github.com/ceph/ceph/blob/master/src/osd/PG.cc#L6958

@yuriw
Copy link
Contributor

yuriw commented Jul 20, 2018

@tchaikov
--- pr 19036 --- pulling https://github.com/tchaikov/ceph.git branch wip-22043
remote: Counting objects: 12, done.
remote: Total 12 (delta 11), reused 11 (delta 11), pack-reused 1
Unpacking objects: 100% (12/12), done.
From https://github.com/tchaikov/ceph

  • branch wip-22043 -> FETCH_HEAD
    Auto-merging src/osd/OSD.h
    Auto-merging src/osd/OSD.cc
    CONFLICT (file/directory): There is a directory with name src/dmclock in f81a715. Adding src/dmclock as src/dmclock~HEAD
    Adding qa/suites/rbd/mirror/cluster
    Adding qa/suites/rbd/mirror/base
    Auto-merging qa/suites/rados/singleton/all/recovery-preemption.yaml
    CONFLICT (content): Merge conflict in qa/suites/rados/singleton/all/recovery-preemption.yaml
    Automatic merge failed; fix conflicts and then commit the result.
    Traceback (most recent call last):
    File "/home/yuriw/wip_master/src/script/build-integration-branch", line 62, in
    assert not r
    AssertionError

@stale
Copy link

stale bot commented Oct 17, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Oct 17, 2018
@neha-ojha
Copy link
Member

@tchaikov this might be useful for https://trello.com/c/lQTxKQow/408-osd-make-recovery-limits-separate-from-osdmaxbackfills, could you please rebase this.

@stale stale bot removed the stale label Oct 19, 2018
@tchaikov tchaikov self-assigned this Oct 20, 2018
@stale
Copy link

stale bot commented Dec 19, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Dec 19, 2018
@liewegas liewegas closed this Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants