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

xds: enable Outlier Detection by default #5673

Merged
merged 1 commit into from Sep 30, 2022

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented Sep 28, 2022

RELEASE NOTES:

  • xds: enable Outlier Detection by default

@zasweq zasweq added this to the 1.50 Release milestone Sep 28, 2022
@zasweq zasweq force-pushed the outlier-detection-default-true branch from 39ae0b2 to 3109845 Compare September 29, 2022 02:47
@zasweq zasweq force-pushed the outlier-detection-default-true branch from 3109845 to ea476a5 Compare September 29, 2022 02:53
@zasweq zasweq requested a review from easwars September 29, 2022 03:03
// "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "true".
XDSOutlierDetection = strings.EqualFold(os.Getenv(outlierDetectionSupportEnv), "true")
// enabled, which can be disabled by setting the environment variable
// "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "false".
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We haven't done this in the past, but at some point we discussed this. We should add the release that we are enabling it from, and add a TODO saying at what release we should completely get rid of the env var. Without this, we will have an ever growing list of env vars here and sprinkled everywhere else in the code.

func TestBuildPriorityConfig(t *testing.T) {
gotConfig, gotAddrs, _ := buildPriorityConfig([]priorityConfig{
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we retain this test and run it with the env var set to "false"?

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with removing it given we would like to eliminate the env var entirely in a few weeks.

@dfawley dfawley changed the title xds: Changed Outlier Detection Env Var to true by default xds: enable Outlier Detection by default Sep 30, 2022
func TestBuildPriorityConfig(t *testing.T) {
gotConfig, gotAddrs, _ := buildPriorityConfig([]priorityConfig{
Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with removing it given we would like to eliminate the env var entirely in a few weeks.

@zasweq zasweq merged commit d83070e into grpc:master Sep 30, 2022
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants