Skip to content

[CLE] Parallelize lease candidate ping order #129814

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

Closed
Jefftree opened this issue Jan 24, 2025 · 3 comments · Fixed by #130533
Closed

[CLE] Parallelize lease candidate ping order #129814

Jefftree opened this issue Jan 24, 2025 · 3 comments · Fixed by #130533
Assignees
Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@Jefftree
Copy link
Member

In the leader election controller: https://github.com/kubernetes/kubernetes/blob/master/pkg/controlplane/controller/leaderelection/leaderelection_controller.go#L267-L293, candidates are iterated through sequentially and LeaseCandidates.Update() is a blocking operation that is also called sequentially. This can lead to cases where certain candidates do not have enough time to respond. We should parallelize this entire operation and iterate through the list in a random order to ensure every candidate has a fair chance of responding.

/cc @Henrywu573
/triage accepted
/sig api-machinery

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Jan 24, 2025
tchap added a commit to tchap/kubernetes that referenced this issue Jan 24, 2025
@tchap
Copy link
Contributor

tchap commented Jan 24, 2025

Sorry, linked this issue by accident when playing with some simple issues to fix in my spare time.

Do you really need to randomize the order? The concurrent requests don't block each other. I guess something like tchap@415c889 could suffice?

@Henrywu573
Copy link
Contributor

/assign

@Jefftree
Copy link
Member Author

Sorry, linked this issue by accident when playing with some simple issues to fix in my spare time.

Do you really need to randomize the order? The concurrent requests don't block each other. I guess something like tchap@415c889 could suffice?

cc @sttts. I think it makes sense that randomization is not strictly necessary if we parallelize. We should be able to guarantee that each candidate has a fair chance of acking with just the parallelization.

@Jefftree Jefftree changed the title [CLE] Randomize lease candidate ping order and parallelize [CLE] Parallelize lease candidate ping order Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
4 participants