Skip to content
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

[MESH-3036] - Set consecutive5xxErrors to 0 #288

Merged
merged 2 commits into from
May 5, 2023

Conversation

vinay-g
Copy link
Collaborator

@vinay-g vinay-g commented May 4, 2023

ConsecutiveGatewayErrors in DestinationRule do not take affect the value is higher that consecutive5xxErrors.

In envoy consecutive5xxErrors value is set to 5 by default.

consecutive5xxErrors needs to be set to 0, to explicitly disable the 5XX error outlier detection to kick in.

Signed-off-by: vgonuguntla <vinay_gonuguntla@intuit.com>
@vinay-g vinay-g force-pushed the oss-MESH-3036 branch 3 times, most recently from 035f55b to 0fcb247 Compare May 4, 2023 08:29
Signed-off-by: vgonuguntla <vinay_gonuguntla@intuit.com>
@itsLucario
Copy link
Collaborator

itsLucario commented May 4, 2023

LGTM.

@vinay-g WDYT about reducing the base_ejection_time and setting max_ejection_time to 300s. So that outlier doesn't become a bottleneck! As of now, we are not setting max_ejection_time. (We can take this up separately)
Refs:
https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/outlier#ejection-algorithm

https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/outlier_detection.proto#envoy-v3-api-field-config-cluster-v3-outlierdetection-max-ejection-time

@vinay-g
Copy link
Collaborator Author

vinay-g commented May 4, 2023

@itsLucario Yup will put that in a separate PR, have to discuss with Anil on that also need to discuss setting the maxEjectionPercent. Since the 100% will cause blackout in multi region scenarios'

Copy link
Contributor

@aattuluri aattuluri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@vinay-g vinay-g merged commit 26e0f7e into istio-ecosystem:master May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants