-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add support for drop category policy and reporting #3894
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 all commits
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 |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ option java_generic_services = true; | |
|
|
||
| import "envoy/api/v2/discovery.proto"; | ||
| import "envoy/api/v2/endpoint/endpoint.proto"; | ||
| import "envoy/type/percent.proto"; | ||
|
|
||
| import "google/api/annotations.proto"; | ||
|
|
||
|
|
@@ -50,12 +51,35 @@ message ClusterLoadAssignment { | |
|
|
||
| // Load balancing policy settings. | ||
| message Policy { | ||
| // Percentage of traffic (0-100) that should be dropped. This | ||
| // action allows protection of upstream hosts should they unable to | ||
| // recover from an outage or should they be unable to autoscale and hence | ||
| // overall incoming traffic volume need to be trimmed to protect them. | ||
| // [#v2-api-diff: This is known as maintenance mode in v1.] | ||
| double drop_overload = 1 [(validate.rules).double = {gte: 0, lte: 100}]; | ||
| reserved 1; | ||
|
|
||
| message DropOverload { | ||
| // Identifier for the policy specifying the drop. | ||
| string category = 1 [(validate.rules).string.min_bytes = 1]; | ||
|
|
||
| // Percentage of traffic that should be dropped for the category. | ||
| envoy.type.Percent drop_percentage = 2; | ||
|
Member
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. @vishalpowar I'm a little late here, but I would recommend using
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 can do that in another PR (this one is already merged) unless there is any restriction on making such changes in two different PRs.
Member
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. @vishalpowar I would just do a follow up PR, it's fine to change it now since it just merged and no one is using it, unless there are objections from others, but we can discuss that in the follow up.
Member
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. Yeah, sure, |
||
| } | ||
| // Action to trim the overall incoming traffic to protect the upstream | ||
| // hosts. This action allows protection in case the hosts are unable to | ||
| // recover from an outage, or unable to autoscale or unable to handle | ||
| // incoming traffic volume for any reason. | ||
| // | ||
| // At the client each category is applied one after the other to generate | ||
| // the 'actual' drop percentage on all outgoing traffic. For example: | ||
| // | ||
| // .. code-block:: json | ||
| // | ||
| // { "drop_overloads": [ | ||
| // { "category": "throttle", "drop_percentage": 60 } | ||
| // { "category": "lb", "drop_percentage": 50 } | ||
| // ]} | ||
| // | ||
| // The actual drop percentages applied to the traffic at the clients will be | ||
| // "throttle"_drop = 60% | ||
| // "lb"_drop = 20% // 50% of the remaining 'actual' load, which is 40%. | ||
| // actual_outgoing_load = 20% // remaining after applying all categories. | ||
| repeated DropOverload drop_overloads = 2; | ||
| } | ||
|
|
||
| // Load balancing policy settings. | ||
|
|
||
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.
I gather this was never implemented, but do we need to keep this in place (but marked deprecated) until 1.8.0 on the off chance that someone is setting it?
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.
I think it's reasonable to skip it. If someone is setting a hidden/unimplemented field, I don't think we need to be applying the policy. Maybe we should clarify it to make this explicit if it isn't already.