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

Don't hang Envoy when result set is empty #5384

Closed
wants to merge 1 commit into from
Closed

Conversation

banks
Copy link
Member

@banks banks commented Feb 25, 2019

As noted in #4868 we can sometimes cause Envoy to hang if one or more upstream has no instances available since Envoy won't continue processing xDS for listeners until it has all the endpoints resolved they'd be proxying to.

I assume that this is correct and that by getting an explicit empty result Envoy will continue to resolve the config and just fail with a 503 if that upstream is connected.

TODO

  • actually test this with Envoy
  • verify that it doesn't fail in some new way when a partial snapshot is delivered (i.e. before some resource has resolved)

As noted in #4868 we can sometimes cause Envoy to hang if one or more upstream has no instances available since Envoy won't continue processing xDS for listeners until it has all the endpoints resolved they'd be proxying to.

I _assume_ that this is correct and that by getting an explicit empty result Envoy will continue to resolve the config and just fail with a 503 if that upstream is connected.

This needs testing though and also ensuring that it doesn't cause any other side-effects.
@banks banks added the wip label Feb 25, 2019
@banks banks requested a review from a team February 25, 2019 15:18
@banks
Copy link
Member Author

banks commented Mar 21, 2019

Superceded by #5499

@banks banks closed this Mar 21, 2019
@banks banks deleted the fix-envoy-hang branch March 21, 2019 16:03
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.

1 participant