-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Optimize ancestor queue resource updates using delta propagation in capacity plugin #4711
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: wangdongyang1 <[email protected]>
|
@kingeasternsun: The label(s) DetailsIn response to this:
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. |
Summary of ChangesHello @kingeasternsun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to enhance the performance of the scheduler's capacity plugin by optimizing how resource updates are handled for hierarchical queues. By implementing a delta propagation mechanism, the system avoids costly full recalculations of ancestor queue resources, leading to more efficient updates and improved scalability, particularly in complex queue hierarchies. This change is designed to maintain existing functional behavior while significantly boosting computational efficiency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a valid optimization to the capacity plugin by pre-calculating resource deltas for ancestor queues. This change effectively reduces redundant computations within the loop, improving performance, especially in deep hierarchical queue structures. The implementation is correct and follows the intended logic. However, there's a subtle issue with the re-use of delta variables which could lead to bugs if the Add method's contract changes in the future. I've added a comment with a suggestion to make the code more robust against such changes.
|
/lgtm |
|
/cc @JesseStutler |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JesseStutler The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind improvement
What this PR does / why we need it:
This PR optimizes the resource update mechanism for ancestor queues in hierarchical queue structures within the capacity plugin.
Optimization Details:
Performance Benefits:
The optimization applies to four resource metrics:
allocated: Resources currently allocated to podsrequest: Total resources requestedinqueue: Resources waiting in queueelastic: Elastic resources availableWhich issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?