add gomaxprocs limit, return node fit error and pod QoS in advance

[fanhaouu]

This PR aims to address the following three issues：

1. The default value of GOMAXPROCS is the number of CPU cores of the machine. After GOMAXPROCS is higher than the number of truly usable cores, the Go scheduler will keep switching OS threads, which will affect the performance of descheduler program;
2. There are many filter policies in the current node fit, similar to the filter plugin in the scheduler. The correct logic should be: if any plugin does not meet the conditions, it should be terminated in advance. The current check logic is incomprehensible, and it actually performs all filter check logic, which is not only time-consuming, but also very unnecessary.(https://github.com/kubernetes-sigs/descheduler/blob/master/pkg/descheduler/node/node.go#L107)
3. If the pod's Qos has a value, return the QoS value early. For pods evicted within the descheduler, they are typically already scheduled onto nodes, and in most cases, the QoS value is already present on the pod's status. Therefore, it is unnecessary to start the determination process from scratch.

[k8s-ci-robot]

Hi @fanhaouu. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

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-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign a7i for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[fanhaouu]

@a7i ,master, can you help me review this pr?

[a7i]

Hi @fanhaouu great contribution. Going to copy some of the maintainers for feedback as well:
/cc @jklaw90 @ingvagabund @damemi

My feedback:

• regarding point 1: For GOMAXPROCS, as far as I know, we don't run any goroutines in Descheduler. How does this help?

• regarding point 2: The idea is to present all NodeFit predicates. The predicate check is not in any particular order, so returning the first one may not present the whole picture to the cluster operator. In a cluster of 35k pods / 15k deployments, a Descheduler run takes a few (single digit) seconds. Given that the shortest frequency can be a minute, I'm not convinced this optimization is worth it. What do you think?

• regarding point 3: I like it, that's a great change!

• overall: if you could split this into 3 PRs, I think it would make it easier to provide feedback and request changes.

[fanhaouu]

Hi @fanhaouu great contribution. Going to copy some of the maintainers for feedback as well: /cc @jklaw90 @ingvagabund @damemi

My feedback:

• regarding point 1: For GOMAXPROCS, as far as I know, we don't run any goroutines in Descheduler. How does this help?
• regarding point 2: The idea is to present all NodeFit predicates. The predicate check is not in any particular order, so returning the first one may not present the whole picture to the cluster operator. In a cluster of 35k pods / 15k deployments, a Descheduler run takes a few (single digit) seconds. Given that the shortest frequency can be a minute, I'm not convinced this optimization is worth it. What do you think?
• regarding point 3: I like it, that's a great change!
• overall: if you could split this into 3 PRs, I think it would make it easier to provide feedback and request changes.

Hi @fanhaouu great contribution. Going to copy some of the maintainers for feedback as well: /cc @jklaw90 @ingvagabund @damemi

My feedback:

• regarding point 1: For GOMAXPROCS, as far as I know, we don't run any goroutines in Descheduler. How does this help?
• regarding point 2: The idea is to present all NodeFit predicates. The predicate check is not in any particular order, so returning the first one may not present the whole picture to the cluster operator. In a cluster of 35k pods / 15k deployments, a Descheduler run takes a few (single digit) seconds. Given that the shortest frequency can be a minute, I'm not convinced this optimization is worth it. What do you think?
• regarding point 3: I like it, that's a great change!
• overall: if you could split this into 3 PRs, I think it would make it easier to provide feedback and request changes.

master, thank you for your reply.

point1. At present, descheduler has some goroutine, but the number is not very large, so gomaxproc can be added or not, but the current go runtime cannot recognize the container environment, I think it is better to add it. For example, jvm runtime has long supported container environments; (As descheduler in our company has a lot of goroutine, the performance can be improved significantly after adding this restriction, so I didn't remove this submission from the pr)

point2. Because the default loop time is 5min, this time can be adjusted. The larger the cluster, the longer the nodeFit time, so the full loop time can easily exceed 5min. What if the user sets the loop time to 1 minute? Comparing one by one I think is very, very unnecessary, too time-consuming, we should be the same as the filter policy in the scheduler.

In short, if we can make the program better, faster, without affecting any functionality, I think it makes a lot of sense.

We are also developing a descheduler cache mechanism internally. Currently, descheduler pulls weight data for filtering at policy runtime, which can actually cache a good part of the data in advance, just like the cache in the scheduler. If our company is stable online, I will also contribute to our descheduler community. We can review it together then.

[ingvagabund]

As Amir mentioned would you please break the PR into three separate PRs? Some of the suggested changes deserves a dedicated discussion. Wrt. NodeFit I am in the process of composing a KEP: #1421. This sounds like a good use case to include in the proposal.

[fanhaouu]

hi, masters, i have split this into 3 PRs, looking forward to your feedback:

1. add GOMAXPROCS limit #1434
2. return pod qos in advance #1435
3. return node fit error in advance #1436

/cc @a7i @jklaw90 @ingvagabund @damemi

[k8s-ci-robot]

PR needs rebase.

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-sigs/prow repository.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[a7i]

/close

it's already split into 3 PRs

[k8s-ci-robot]

@a7i: Closed this PR.

In response to this:

/close

it's already split into 3 PRs

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-sigs/prow repository.