Update Envoy to 888e0e28900a470df448c65d7b99d8065fd60251#331
Merged
htuch merged 6 commits intoenvoyproxy:masterfrom May 11, 2020
Merged
Update Envoy to 888e0e28900a470df448c65d7b99d8065fd60251#331htuch merged 6 commits intoenvoyproxy:masterfrom
htuch merged 6 commits intoenvoyproxy:masterfrom
Conversation
Saving state - work in progress This update is a bit more challenging then usual, as the legacy connection pool has been axed out. This means we need to move towards the new pool implementations, and tackle some challenges associated to that. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
- Update to make the new h1 pool work with lru/mru connection re-use & connection prefetching - add todos after analysing what needs to be done for h2 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Retains the experimental h2 feature based on the new h2 pool. Behavior has changed. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Update for changed RuntimeImpl constructor Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
oschaaf
changed the title
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Member
Author
|
/cc @yanavlasov |
oschaaf
commented
May 9, 2020
| case Envoy::Http::ConnectionPool::PoolFailureReason::RemoteConnectionFailure: | ||
| benchmark_client_stats_.pool_connection_failure_.inc(); | ||
| break; | ||
| case Envoy::Http::ConnectionPool::PoolFailureReason::Timeout: |
Member
Author
There was a problem hiding this comment.
I will add a TODO to track these, as well as add separate counters for the combined connection failure counter above.
yanavlasov
previously approved these changes
May 10, 2020
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Member
Author
|
I had to amend test expectations in 571c399 to allow for a behavior that showed up in the earlier CI tsan run. I've been looking into that, and this looks like it is stock Envoy behavior to exceed the configured connection limits in some scenarios. |
Member
Author
|
As a next step to this, to enhance support for supporting multiple http/2 connections, we could consider exposing the max concurrent streams setting + warn/deprecate the experimental flag which just caps that to |
yanavlasov
approved these changes
May 11, 2020
htuch
approved these changes
May 11, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This Envoy update is a bit more involved, because it forced moving away from the
legacy connection pools it had, which got eliminated from the code base. So we migrate
to Envoy's new pool models.
The h1 pool migrates seamlessly to the new model with minor changes.
The experimental h2 connection pool that supports multiple connections gets
dropped. We now offer multiple connection support based on the new H2 pool.
Behavior has changed, but there was a warning about that in the description of
the
--experimentalflag (see changes to the test associated to our earlier experimental h2pool)
It should now be possible to add new options to add better support for h2 & multiple connections,
and deprecate or remove the
--experimentalflag for that.Signed-off-by: Otto van der Schaaf oschaaf@we-amp.com