-
Notifications
You must be signed in to change notification settings - Fork 502
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
tidb-scheduler: implement kube-scheduler extender preempt verb #2510
Conversation
5a9d0c4
to
2cf3f8c
Compare
dd41099
to
6e274a7
Compare
/run-e2e-tests |
cc @mightyguava can you try this patch in your test environment? here are instructions on how to build a custom image:
then you can update the tidb-operator image of tidb-scheduler deployment. |
I'm still not seeing the low priority pods get preempted. Here are the logs from the current run. tidb-scheduler.log
kube-scheduler.log
Could you elaborate here? I'm not familiar with the scheduler phases. We want these low priority overprovisioning pods to be preempted so that PD (and other pods) can be scheduled on them. Why would we want to exclude these nodes from preemption?
|
@mightyguava I forgot to tell you that scheduler policy
add Can you update the config map and try again? |
I made the config change and it's still not working.
There's an error in the kube-scheduler logs below as you can see but there is no more info what the error is about. :-( configmap/tidb-scheduler-policy
tikv-scheduler.log
kube-scheduler.log
|
I guess the pod
The basic scheduler workflow in 1.14.x is like this:
at before, our extender (tidb-scheduler) does not implement preempt API, all nodes are chosen by kube-scheduler.
an invalid node (ip-10-136-135-87.us-west-2.compute.internal) is nominated but this node already has a PD running and is rejected by tidb-scheduler Filter API.
after this PR, |
Thanks! can you show the output of this command? I'd like know the current status:
|
I'll test with your Kubernetes version (1.14.9-eks) to see why the scheduler emits these errors. Thanks for the feedback!
|
Here's the kube-scheduler.log
(updated with more logs) |
|
The potential nodes are sent to the extender but not parsed from JSON string correctly. There is a bug in Kubernetes 1.16.0 and before that the JSON tag in https://github.com/kubernetes/kubernetes/blob/v1.16.0/pkg/scheduler/api/v1/types.go#L270 |
hi, @mightyguava @tirsen PR is updated, can you build the image and try again? I've verified it manually with this example, the scheduler evicts all overprovisioning pods and schedule pods onto the nodes. We will add an integration or e2e test for this scenario. |
Yes! This works! This is awesome. The overprovisioning pods were successfully preempted, the tidb pods scheduled, and new ec2 nodes were created to run the overprovisioning pods |
Great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-e2e-tests |
1 similar comment
/run-e2e-tests |
/merge |
/run-all-tests |
@cofyc merge failed. |
/run-e2e-tests |
/run-cherry-picker |
Signed-off-by: sre-bot <[email protected]>
cherry pick to release-1.1 in PR #2555 |
What problem does this PR solve?
fixes #1968
What is changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
Does this PR introduce a user-facing change?: