feat:support slow start mode #2219

tmsnan · 2023-11-20T08:10:03Z

What type of PR is this?
Support slow start model

What this PR does / why we need it:

Which issue(s) this PR fixes:

Releated:
#1902
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#config-cluster-v3-cluster-slowstartconfig

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

codecov · 2023-11-20T08:20:31Z

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (571f8bf) 64.49% compared to head (f530015) 64.50%.

Files	Patch %	Lines
internal/ir/zz_generated.deepcopy.go	0.00%	26 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2219      +/-   ##
==========================================
+ Coverage   64.49%   64.50%   +0.01%     
==========================================
  Files         111      111              
  Lines       15482    15542      +60     
==========================================
+ Hits         9985    10026      +41     
- Misses       4881     4900      +19     
  Partials      616      616

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

api/v1alpha1/loadbalancer_types.go

zirain · 2023-11-20T08:51:46Z

can you share the use case for this? add an e2e test for this if possible?
there're tons of feautes envoy support, EG won't expose all of them.
IMO, most of the edge case can be achieved by EnvoyPath?

shawnh2

generally LGTM, just some nits and please fix ci

api/v1alpha1/loadbalancer_types.go

internal/xds/translator/cluster.go

internal/gatewayapi/backendtrafficpolicy.go

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

tmsnan · 2023-11-20T09:38:34Z

can you share the use case for this? add an e2e test for this if possible? there're tons of feautes envoy support, EG won't expose all of them. IMO, most of the edge case can be achieved by EnvoyPath?

I will try to implement this e2e test through indicator comparison later.
Slow-start mode is useful for certain backend applications that might otherwise be overwhelmed during warm-up. Such applications may respond slowly to requests or return errors immediately after a pod starts or a container restarts. Using a slow-start configuration to gradually increase traffic to the most recently started service endpoint can reduce the impact of this behavior on users. So I think it is necessary to provide this feature in load balancing. Secondly, the API does not provide complicated configuration (like envoy) to enable slow start. It only enables the function by configuring the slow response time window. If more fine-grained use is required, EnvoyPath is suitable.

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

api/v1alpha1/loadbalancer_types.go

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

tmsnan · 2023-11-21T12:01:26Z

/retest

api/v1alpha1/loadbalancer_types.go

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

internal/gatewayapi/backendtrafficpolicy.go

internal/ir/xds.go

arkodg · 2023-11-22T03:23:51Z

internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.in.yaml

@@ -91,3 +110,33 @@ backendTrafficPolicies:
      type: ConsistentHash
      consistentHash:
        type: SourceIP
+- apiVersion: gateway.envoyproxy.io/v1alpha1


instead of creating a new route, can we just add this slowStart config to the existing config ?

The existing route has been used to test ConsistentHash, so a new route needs to be added

internal/xds/translator/cluster.go

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

tmsnan · 2023-11-23T11:53:22Z

/retest

zhaohuabing · 2023-11-24T00:10:36Z

internal/ir/xds.go

+// +k8s:deepcopy-gen=true
+type SlowStart struct {
+	// Window defines the duration of the warm up period for newly added host.
+	Window *metav1.Duration `json:"window" yaml:"window"`


Should Window be defined as a pointer here? It's required.

I suggested this, to make it easier for adding if checks within xds code

Do you mean here? Can the second if be removed since window is a mandatory field?

if args.loadBalancer.RoundRobin.SlowStart != nil { if args.loadBalancer.RoundRobin.SlowStart.Window != nil { cluster.LbConfig = &clusterv3.Cluster_RoundRobinLbConfig_{ RoundRobinLbConfig: &clusterv3.Cluster_RoundRobinLbConfig{ SlowStartConfig: &clusterv3.Cluster_SlowStartConfig{ SlowStartWindow: durationpb.New(args.loadBalancer.RoundRobin.SlowStart.Window.Duration), }, }, } } }

zhaohuabing · 2023-11-24T00:12:37Z

api/v1alpha1/loadbalancer_types.go

+	// Currently only supports linear growth of traffic. For additional details,
+	// see https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#config-cluster-v3-cluster-slowstartconfig
+	// +kubebuilder:validation:Required
+	Window *metav1.Duration `json:"window"`


The same as above. Should Window be defined as a pointer here? It's required.

zhaohuabing

LGTM. Just some small nits.

arkodg · 2023-11-24T00:20:21Z

internal/gatewayapi/backendtrafficpolicy.go

 	case egv1a1.RandomLoadBalancerType:
 		lb = &ir.LoadBalancer{
 			Random: &ir.Random{},
 		}
 	case egv1a1.RoundRobinLoadBalancerType:
 		lb = &ir.LoadBalancer{
-			RoundRobin: &ir.RoundRobin{},
+			RoundRobin: &ir.RoundRobin{
+				SlowStart: &ir.SlowStart{},


this slow start field still exists on line 455, can you rm this ?

Done, my bad, I totally missed it many times.

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

feat:support slow start mode

77f8b0e

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

tmsnan requested a review from a team as a code owner November 20, 2023 08:10

zirain reviewed Nov 20, 2023

View reviewed changes

api/v1alpha1/loadbalancer_types.go Outdated Show resolved Hide resolved

shawnh2 reviewed Nov 20, 2023

View reviewed changes

api/v1alpha1/loadbalancer_types.go Outdated Show resolved Hide resolved

internal/xds/translator/cluster.go Outdated Show resolved Hide resolved

internal/gatewayapi/backendtrafficpolicy.go Outdated Show resolved Hide resolved

fix api and add test

ecf9f44

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

tmsnan added 4 commits November 20, 2023 19:13

fix cel validation and test

77f7358

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

fix gen-check

381cffc

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

fix cel validation

95d15b6

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

Merge branch 'main' into dev-slow-start-mode

141672e

arkodg reviewed Nov 20, 2023

View reviewed changes

api/v1alpha1/loadbalancer_types.go Outdated Show resolved Hide resolved

arkodg reviewed Nov 20, 2023

View reviewed changes

api/v1alpha1/loadbalancer_types.go Outdated Show resolved Hide resolved

tmsnan added 2 commits November 21, 2023 16:37

update slow start api

e913a7b

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

Merge branch 'main' into dev-slow-start-mode

341f2cc

arkodg reviewed Nov 22, 2023

View reviewed changes

api/v1alpha1/loadbalancer_types.go Outdated Show resolved Hide resolved

arkodg reviewed Nov 22, 2023

View reviewed changes

api/v1alpha1/loadbalancer_types.go Outdated Show resolved Hide resolved

update api

00fde5d

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

arkodg reviewed Nov 22, 2023

View reviewed changes

internal/gatewayapi/backendtrafficpolicy.go Outdated Show resolved Hide resolved

arkodg reviewed Nov 22, 2023

View reviewed changes

internal/gatewayapi/backendtrafficpolicy.go Show resolved Hide resolved

arkodg reviewed Nov 22, 2023

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

arkodg reviewed Nov 22, 2023

View reviewed changes

internal/xds/translator/cluster.go Outdated Show resolved Hide resolved

tmsnan added 3 commits November 22, 2023 17:54

optimize some code

ac46a68

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

Merge branch 'main' into dev-slow-start-mode

c191054

merge main

2569e75

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

zhaohuabing reviewed Nov 24, 2023

View reviewed changes

arkodg reviewed Nov 24, 2023

View reviewed changes

remove unused code

f530015

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>

arkodg approved these changes Nov 24, 2023

View reviewed changes

zirain approved these changes Nov 24, 2023

View reviewed changes

zirain merged commit 73fbd62 into envoyproxy:main Nov 24, 2023
18 checks passed

tmsnan deleted the dev-slow-start-mode branch December 6, 2023 11:29

Xunzhuo mentioned this pull request Dec 7, 2023

Support slow start model #1902

Closed

Xunzhuo added the road-to-ga label Dec 7, 2023

Xunzhuo added this to the v1.0.0-rc1 milestone Dec 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:support slow start mode #2219

feat:support slow start mode #2219

tmsnan commented Nov 20, 2023

codecov bot commented Nov 20, 2023 •

edited

Loading

zirain commented Nov 20, 2023

shawnh2 left a comment

tmsnan commented Nov 20, 2023

tmsnan commented Nov 21, 2023

arkodg Nov 22, 2023

tmsnan Nov 22, 2023

tmsnan commented Nov 23, 2023

zhaohuabing Nov 24, 2023 •

edited

Loading

arkodg Nov 24, 2023

zhaohuabing Nov 24, 2023

zhaohuabing Nov 24, 2023 •

edited

Loading

zhaohuabing left a comment

arkodg Nov 24, 2023

tmsnan Nov 24, 2023

feat:support slow start mode #2219

feat:support slow start mode #2219

Conversation

tmsnan commented Nov 20, 2023

codecov bot commented Nov 20, 2023 • edited Loading

Codecov Report

zirain commented Nov 20, 2023

shawnh2 left a comment

Choose a reason for hiding this comment

tmsnan commented Nov 20, 2023

tmsnan commented Nov 21, 2023

arkodg Nov 22, 2023

Choose a reason for hiding this comment

tmsnan Nov 22, 2023

Choose a reason for hiding this comment

tmsnan commented Nov 23, 2023

zhaohuabing Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

arkodg Nov 24, 2023

Choose a reason for hiding this comment

zhaohuabing Nov 24, 2023

Choose a reason for hiding this comment

zhaohuabing Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

zhaohuabing left a comment

Choose a reason for hiding this comment

arkodg Nov 24, 2023

Choose a reason for hiding this comment

tmsnan Nov 24, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 20, 2023 •

edited

Loading

zhaohuabing Nov 24, 2023 •

edited

Loading

zhaohuabing Nov 24, 2023 •

edited

Loading