-
Notifications
You must be signed in to change notification settings - Fork 7k
[Core][Autoscaler] Update the Autoscaler Resource Requests Data Model for Scheduling for Label Selector #51771
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
Changes from 1 commit
c7e646e
8966a65
d67f91c
44c5218
afb7a8c
2f8892a
64a4abd
2605e72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,27 +51,71 @@ message PlacementConstraint { | |
| optional AffinityConstraint affinity = 2; | ||
| } | ||
|
|
||
| // The type of operator to use for the label constraint. | ||
| enum LabelOperator { | ||
| LABEL_OPERATOR_UNSPECIFIED = 0; | ||
| // This is to support equality or in semantics. | ||
| LABEL_OPERATOR_IN = 1; | ||
| // This is to support not equal or not in semantics. | ||
| LABEL_OPERATOR_NOT_IN = 2; | ||
| } | ||
|
|
||
| // A node label constraint with a key, one or a list of values and an operator. | ||
| message LabelConstraint { | ||
| // The key of the label | ||
| string label_key = 1; | ||
| // The values to check against. | ||
| repeated string label_values = 2; | ||
| // The operator to use for the label constraint. | ||
| LabelOperator operator = 3; | ||
MengjinYan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // A list of node label constraints to be used for specify the label requirements in a | ||
| // resource request. | ||
MengjinYan marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| message LabelSelector { | ||
| // The list of node label constraints with AND semantics. | ||
| repeated LabelConstraint label_constraints = 1; | ||
| } | ||
|
|
||
| message ResourceRequest { | ||
| // resource requirements for the request. | ||
| map<string, double> resources_bundle = 1; | ||
| // placement constraint for the request. multiple constraints | ||
| // form AND semantics. | ||
| repeated PlacementConstraint placement_constraints = 2; | ||
| // The node label requirements for the request. Multiple label selectors are for | ||
| // fallback mechanism. When trying to find a node that satisfies the label | ||
| // requirements, the first label selector should be tried first, if not found, | ||
| // the second label selector should be tried, and so on. | ||
| repeated LabelSelector label_selectors = 3; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how would this work with coordinated fallback between resources and labels? That is, overriding resource requirements depending on the label selector that's selected.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't necessarily need to make the change to support this immediately if there is a path to support it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My idea is that if we want to do coordinated fallback between resources and labels, we can do similar things as the what we are doing with the placement group bundles:
In this way, the newly added resource_selectors and the existing label_selectors can work together to support the coordinated fallback mechanism. What do you think?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sense to me -- we should align the interfaces for PG and regular tasks/actors as much as possible
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think we should do it now, or can be added when we take on the work to do resource fallback?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can do that when we do the resource fallback work. |
||
| } | ||
|
|
||
| message ResourceRequestByCount { | ||
| ResourceRequest request = 1; | ||
| int64 count = 2; | ||
| } | ||
|
|
||
| // All bundles in the same resource request require gang | ||
| // A bundle selector used to specify the resource bundles that should be | ||
| // allocated together. All bundles in the same resource request require gang | ||
| // allocation semantics: they should be allocated all or nothing. | ||
| message BundleSelector { | ||
| // The list of resource requests that should be allocated together. | ||
| repeated ResourceRequest resource_requests = 1; | ||
| } | ||
|
Comment on lines
+104
to
+107
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you provide an example of how this would be used for fallback?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually the note about the fallback mechanism for I also just added the more notes in Wondering if this is something you were looking for? |
||
|
|
||
| message GangResourceRequest { | ||
| // a map from bundles to the number of bundles requested. | ||
| // DEPRECATED: bundle_selector should be used instead so that we can support fallback | ||
| // mechanism. | ||
| repeated ResourceRequest requests = 1; | ||
| // Metadata associated with the request for observability, | ||
| // e.g. placement group's strategy. | ||
| string details = 2; | ||
| // The bundle requests. Multiple bundle selectors are for fallback mechanism. | ||
| // When trying to find nodes that satisfies the bundle selector, the first bundle | ||
| // selector should be tried first, if not found, the second bundle selector should be | ||
| // tried, and so on. | ||
| repeated BundleSelector bundle_selectors = 3; | ||
| } | ||
|
|
||
| // Cluster resource constraint represents minimal cluster size requirement, | ||
|
|
||
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.
so exist or not exist are not supported at the first place.
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.
Yes from my understanding here: https://docs.google.com/document/d/1DKhPuZERlLbsU4TIQZZ6POCsm1pVMBgN_yn5r0OmaDI/edit?tab=t.0#heading=h.428qclpgsw2d
cc: @edoakes to confirm
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.
sg. The pr lg. Will let Janet take a look as well
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.
correct, we can add in the future