-
Notifications
You must be signed in to change notification settings - Fork 606
feat(loadbalancer): Add LoadBalancerType Client Side Weighted Round Robin #7407
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: main
Are you sure you want to change the base?
Changes from all commits
971c3f1
a9d01f0
63e8b2f
522534b
4512f9d
dd5d285
1101c1d
14d1d59
8bce886
0d65b01
a479e67
777a1c1
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 |
|---|---|---|
|
|
@@ -11,15 +11,17 @@ import gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" | |
| // +union | ||
| // | ||
| // +kubebuilder:validation:XValidation:rule="self.type == 'ConsistentHash' ? has(self.consistentHash) : !has(self.consistentHash)",message="If LoadBalancer type is consistentHash, consistentHash field needs to be set." | ||
| // +kubebuilder:validation:XValidation:rule="self.type in ['Random', 'ConsistentHash'] ? !has(self.slowStart) : true ",message="Currently SlowStart is only supported for RoundRobin and LeastRequest load balancers." | ||
| // +kubebuilder:validation:XValidation:rule="self.type == 'ConsistentHash' ? !has(self.zoneAware) : true ",message="Currently ZoneAware is only supported for LeastRequest, Random, and RoundRobin load balancers." | ||
| // +kubebuilder:validation:XValidation:rule="self.type == 'ClientSideWeightedRoundRobin' ? has(self.clientSideWeightedRoundRobin) : !has(self.clientSideWeightedRoundRobin)",message="If LoadBalancer type is ClientSideWeightedRoundRobin, clientSideWeightedRoundRobin field needs to be set." | ||
| // +kubebuilder:validation:XValidation:rule="self.type in ['Random', 'ConsistentHash'] ? !has(self.slowStart) : true ",message="Currently SlowStart is only supported for RoundRobin, LeastRequest, and ClientSideWeightedRoundRobin load balancers." | ||
| // +kubebuilder:validation:XValidation:rule="self.type in ['ConsistentHash', 'ClientSideWeightedRoundRobin'] ? !has(self.zoneAware) : true ",message="Currently ZoneAware is only supported for LeastRequest, Random, and RoundRobin load balancers." | ||
| type LoadBalancer struct { | ||
| // Type decides the type of Load Balancer policy. | ||
| // Valid LoadBalancerType values are | ||
| // "ConsistentHash", | ||
| // "LeastRequest", | ||
| // "Random", | ||
| // "RoundRobin". | ||
| // "RoundRobin", | ||
| // "ClientSideWeightedRoundRobin". | ||
| // | ||
| // +unionDiscriminator | ||
| Type LoadBalancerType `json:"type"` | ||
|
|
@@ -29,6 +31,12 @@ type LoadBalancer struct { | |
| // +optional | ||
| ConsistentHash *ConsistentHash `json:"consistentHash,omitempty"` | ||
|
|
||
| // ClientSideWeightedRoundRobin defines the configuration when the load balancer type is | ||
| // set to ClientSideWeightedRoundRobin. | ||
| // | ||
| // +optional | ||
| ClientSideWeightedRoundRobin *ClientSideWeightedRoundRobin `json:"clientSideWeightedRoundRobin,omitempty"` | ||
|
|
||
| // EndpointOverride defines the configuration for endpoint override. | ||
| // When specified, the load balancer will attempt to route requests to endpoints | ||
| // based on the override information extracted from request headers or metadata. | ||
|
|
@@ -39,7 +47,7 @@ type LoadBalancer struct { | |
|
|
||
| // SlowStart defines the configuration related to the slow start load balancer policy. | ||
| // If set, during slow start window, traffic sent to the newly added hosts will gradually increase. | ||
| // Currently this is only supported for RoundRobin and LeastRequest load balancers | ||
| // Supported for RoundRobin, LeastRequest, and ClientSideWeightedRoundRobin load balancers. | ||
| // | ||
| // +optional | ||
| SlowStart *SlowStart `json:"slowStart,omitempty"` | ||
|
|
@@ -51,7 +59,7 @@ type LoadBalancer struct { | |
| } | ||
|
|
||
| // LoadBalancerType specifies the types of LoadBalancer. | ||
| // +kubebuilder:validation:Enum=ConsistentHash;LeastRequest;Random;RoundRobin | ||
| // +kubebuilder:validation:Enum=ConsistentHash;LeastRequest;Random;RoundRobin;ClientSideWeightedRoundRobin | ||
| type LoadBalancerType string | ||
|
|
||
| const ( | ||
|
|
@@ -63,6 +71,8 @@ const ( | |
| RandomLoadBalancerType LoadBalancerType = "Random" | ||
| // RoundRobinLoadBalancerType load balancer policy. | ||
| RoundRobinLoadBalancerType LoadBalancerType = "RoundRobin" | ||
| // ClientSideWeightedRoundRobinLoadBalancerType load balancer policy. | ||
| ClientSideWeightedRoundRobinLoadBalancerType LoadBalancerType = "ClientSideWeightedRoundRobin" | ||
| ) | ||
|
|
||
| // ConsistentHash defines the configuration related to the consistent hash | ||
|
|
@@ -148,6 +158,44 @@ type Cookie struct { | |
| Attributes map[string]string `json:"attributes,omitempty"` | ||
| } | ||
|
|
||
| // ClientSideWeightedRoundRobin defines configuration for Envoy's Client-Side Weighted Round Robin policy. | ||
| // See Envoy proto: envoy.extensions.load_balancing_policies.client_side_weighted_round_robin.v3.ClientSideWeightedRoundRobin | ||
|
Contributor
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. tbh https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/load_balancing_policies/client_side_weighted_round_robin/v3/client_side_weighted_round_robin.proto is not descriptive enough, and doesnt mention how this needs to be instrumented in the upstream, are there other links than can be used
Contributor
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 the docs aren't great... for example out-of-band reporting isn't supported at all yet reading the docs would indicate otherwise. We could contribute some docs changes upstream but agreed that including a reference to how
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. @arkodg what changes would you suggest me to add here? This is a bit more elaborative document https://docs.cloud.google.com/load-balancing/docs/https/applb-custom-metrics although this can also be confusing and gcp specific.
Contributor
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 any docs exist that explain how a backend / server should be enhanced to send the appropriate trailors in the response ?
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. There is a google doc I will have to find it though but nothing stated officially as far I am aware of
Contributor
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 should consider adding one if it doesnt exist 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. Ack, this is good feedback on the docs gaps. I'll work with @efimki on this.
Contributor
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. thanks @AndresGuedez ! Happy to be the beta tester |
||
| type ClientSideWeightedRoundRobin struct { | ||
|
Contributor
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 saw your pr adding slow_start was merged in as well so feel free to include that now if you'd like and we'll get this into v1.7.
Contributor
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 the existing slow_start field be reused ?
Contributor
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. Yep it uses
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. Should we also support the min_weight_percent and aggression now in this MR or in a separate MR?
Contributor
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. in the first iteration, do we need to add all these fields, or can be rely on the defaults that are preset ?
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. So I have removed the other fields which are not currently supported by envoy, but at present I guess the fields being published are important to properly configure the system. For example black out period can be different for different applications based on how much time they take to get the metrics state. We have some apps which does heavy data loading in the beginning and hence their blackout periods are high. I think all of these metrics are application specific and we have in our sytems already configured / tuned them for different services. So since the implementation is already done are there any concerns of why we should not include them?
Contributor
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 as a project, we want to limit the scope of API fields to the ones majority users will configure, trying to simplify the complexity and only add it when needed
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. But that compromises with the configurability as it depends lot on the service being served |
||
| // A given endpoint must report load metrics continuously for at least this long before the endpoint weight will be used. | ||
| // Default is 10s. | ||
| // +optional | ||
| BlackoutPeriod *gwapiv1.Duration `json:"blackoutPeriod,omitempty"` | ||
|
|
||
| // If a given endpoint has not reported load metrics in this long, stop using the reported weight. Defaults to 3m. | ||
| // +optional | ||
| WeightExpirationPeriod *gwapiv1.Duration `json:"weightExpirationPeriod,omitempty"` | ||
|
|
||
| // How often endpoint weights are recalculated. Values less than 100ms are capped at 100ms. Default 1s. | ||
| // +optional | ||
| WeightUpdatePeriod *gwapiv1.Duration `json:"weightUpdatePeriod,omitempty"` | ||
|
|
||
| // ErrorUtilizationPenalty adjusts endpoint weights based on the error rate (eps/qps). | ||
| // This is expressed as a percentage-based integer where 100 represents 1.0, 150 represents 1.5, etc. | ||
| // | ||
| // For example: | ||
| // - 100 => 1.0x | ||
| // - 120 => 1.2x | ||
| // - 200 => 2.0x | ||
| // | ||
| // Note: In the internal IR/XDS configuration this value is converted back to a | ||
| // floating point multiplier (value / 100.0). | ||
| // | ||
| // Must be non-negative. | ||
| // +kubebuilder:validation:Minimum=0 | ||
| // +optional | ||
| ErrorUtilizationPenalty *uint32 `json:"errorUtilizationPenalty,omitempty"` | ||
|
|
||
| // Metric names used to compute utilization if application_utilization is not set. | ||
| // For map fields in ORCA proto, use the form "<map_field>.<key>", e.g., "named_metrics.foo". | ||
| // +optional | ||
| MetricNamesForComputingUtilization []string `json:"metricNamesForComputingUtilization,omitempty"` | ||
| } | ||
|
|
||
| // ConsistentHashType defines the type of input to hash on. | ||
| // +kubebuilder:validation:Enum=SourceIP;Header;Headers;Cookie;QueryParams | ||
| type ConsistentHashType string | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
thoughts on other names involving BackendMetrics or Utilization or ORCA ?
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.
If length is your concern
ClientSideWRRis an abbreviation I've seen used in a few places.I agree the name is confusing but it's hard to rename this as the exclusive ORCA or Utilization lb type because similar lb policies exist such as WrrLocality.
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.
also client side doesnt convey any useful meaning to the user, we refer to downstream as clients in EG
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.
We can change the name, also is there a discussion group or slack channel where we can disucss and close this?