Skip to content

Update preemption policy kep#1096

Closed
xychu wants to merge 1 commit intokubernetes:masterfrom
xychu:add-more-preemption-policy
Closed

Update preemption policy kep#1096
xychu wants to merge 1 commit intokubernetes:masterfrom
xychu:add-more-preemption-policy

Conversation

@xychu
Copy link
Copy Markdown

@xychu xychu commented Jun 11, 2019

As we've have non-preempting(kubernetes/kubernetes#74614) policy, for some cases, users may need non-preemptible PriorityClasses as well.

We could add two more preemption policies to control both preempting and preemptible:

  • PreemptLowerPriority means that pod can preempt other pods with lower priority and can be preempted by other pods with higher priority.
  • PreemptNever means that pod never preempts other pods with lower priority and can be preempted by other pods with higher priority.
  • NonPreemptible means that pod can preempt other pods with lower priority and can not be preempted by other pods with higher priority.
  • NonPreemptiblePreemptNever means that pod can not preempt other pods with lower priority and can not be preempted by other pods with higher priority.

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Welcome @xychu!

It looks like this is your first PR to kubernetes/enhancements 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/enhancements has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 11, 2019
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Hi @xychu. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 11, 2019
@k8s-ci-robot k8s-ci-robot requested review from bsalamat and k82cn June 11, 2019 04:13
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xychu
To complete the pull request process, please assign bsalamat
You can assign the PR to them by writing /assign @bsalamat in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@xychu
Copy link
Copy Markdown
Author

xychu commented Jun 11, 2019

PR: kubernetes/kubernetes#78844

can be preempted by other pods with higher priority.
* PreemptNever means that pod never preempts other pods with lower priority and
can be preempted by other pods with higher priority.
* NonPreemptible means that pod can preempt other pods with lower priority and
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly object to a NonPreemptible policy as it clearly conflicts with high priority classes. It could cause confusion for users, is prone to mis-configuration. If cluster admins are not careful about allocating quota at low priority non-preemptible priorities, these priority classes could create abuse opportunities for users and allow them to hug cluster resources.
An example that could cause a cluster to malfunction: There are some daemon pods with critical priority in a cluster. They cannot be scheduled due to lack of resources. Today, the scheduler will remove any pod with lowest priority in the cluster to create room for such critical priority pods. Now imagine that all of those lower priority pods were non-preemptible. What should the scheduler do? Should it respect the critical priority of the daemons or the fact that lower priority pods are non-preemptible?

For these reasons, I believe we should only allow higher priority pods to yield to lower priority pods, but we should never let victims to decide whether they can be preempted or not.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that abusing NonPreemptible policy would cause resource problems.

The main motivation here is to separate the preemption from priority value. For each priority class, there will be the "value" for scheduling order and "preemption policy" for preemption part.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your proposal, but I think this could cause issues as I explained above.

@fejta-bot
Copy link
Copy Markdown

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 15, 2019
@fejta-bot
Copy link
Copy Markdown

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 15, 2019
@fejta-bot
Copy link
Copy Markdown

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@fejta-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants