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

Implement changes for new added flag of disable_overprovisioning in ClusterLoadAssignment #8093

Closed
jaychenatr opened this issue Aug 29, 2019 · 8 comments
Labels
enhancement Feature requests. Not bugs or questions. no stalebot Disables stalebot from closing an issue
Milestone

Comments

@jaychenatr
Copy link
Contributor

Description:
As a new API change will be introduced for disable_overprovisioning flag in #8080. Changes are required to be implemented accordingly in Envoy. If disable_overprovisioning is set to false(default value), Envoy will behave the same as overprovisioning_factor suggests, Otherwise Envoy need to ignore disable_overprovisioning and stop performing graceful failover between priorities/localities as endpoints become unhealthy.

@mattklein123 mattklein123 added the enhancement Feature requests. Not bugs or questions. label Aug 30, 2019
@mattklein123 mattklein123 added this to the 1.12.0 milestone Aug 30, 2019
@mattklein123 mattklein123 added the no stalebot Disables stalebot from closing an issue label Aug 30, 2019
@jaychenatr jaychenatr removed their assignment Sep 9, 2019
@mattklein123 mattklein123 modified the milestones: 1.12.0, 1.13.0 Oct 10, 2019
@mattklein123 mattklein123 modified the milestones: 1.13.0, 1.14.0 Dec 6, 2019
@markdroth
Copy link
Contributor

@htuch @alyssawilk @snowp

We will definitely need this for 1.14.

@alyssawilk
Copy link
Contributor

Sorry, can you doc up the behavior you are expecting? I think I am not understanding what you are trying to do here.
Do you just want to serve 500s based on the percent unhealthy rather than moving your traffic to other viable endpoints? To me, disable_overprovisioning would be setting the overprovisioning factor to 1 (not over provisioning). Are you trying to disable failover?

@muxi
Copy link

muxi commented Feb 12, 2020

The idea is to disable the failover. When disable_overprovisioning is set to true, Envoy does not automatically failover based on locality health; the xds server takes the responsibility of routing traffics to other localities by pushing new configurations.

I dug a little bit into Envoy code and I guess it's probably enough to

I'm not sure if that is the right direction or if it covers all the necessary changes though. Would love to get some pointer.

@srini100
Copy link

/cc

@mattklein123
Copy link
Member

@muxi that sounds about right. cc @snowp for further guidance.

@snowp
Copy link
Contributor

snowp commented Feb 14, 2020

I think in the past we've suggested people just set their overprovisioning factor really high in order to disable spillover, would this new config option just be equivalent to that?

@muxi
Copy link

muxi commented Feb 14, 2020

You are right. I did not realize that but looking at the code in upstream_impl again it seems to be equivalent (except disable_overprovisioning is an on/off switch which has slightly different semantics). In that case I'll probably leave it as is for now. Thanks for the idea!

Besides that, hope to ask another question: is there any way to configure the default overprovisioning_factor locally for Envoy (e.g. with an environment variable or something)? It looks not likely to me because the default value seems to be a constant but in case you know another way hidden somewhere. Thanks!

@markdroth
Copy link
Contributor

The need for this should be eliminated by #10136. Feel free to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. no stalebot Disables stalebot from closing an issue
Projects
None yet
Development

No branches or pull requests

7 participants