Skip to content

Conversation

zuercher
Copy link
Member

@zuercher zuercher commented Aug 6, 2018

Converts TcpProxy::Filter and WebSocket::WsHandlerImpl to use
Tcp::ConnectionPool to obtain connections. Much of the stats
handling and connection timeouts are handled by the connection
pool.

Stats were manually verified by comparing stats produced by
the tcp_proxy_integration_test with and without the connection
pool change.

This reapplies an earlier changed reverted due to #4065. That
bug is fixed by performing the connection pools state management
before forwarding callbacks.

Risk Level: medium
Testing: unit/integration testing
Docs Changes: n/a
Release Notes: n/a
Fixes: #4065

Signed-off-by: Stephan Zuercher stephan@turbinelabs.io

Converts TcpProxy::Filter and WebSocket::WsHandlerImpl to use
Tcp::ConnectionPool to obtain connections. Much of the stats
handling and connection timeouts are handled by the connection
pool.

Stats were manually verified by comparing stats produced by
the tcp_proxy_integration_test with and without the connection
pool change.

This reapplies an earlier changed reverted due to envoyproxy#4066. That
bug is fixed by performing the connection pools state management
before forwarding callbacks.

*Risk Level*: medium
*Testing*: unit/integration testing
*Docs Changes*: n/a
*Release Notes*: n/a
*Fixes*: envoyproxy#4066

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
…arding

callbacks and tracking whether the connection has been invalidated
or not (allowing the callback to use the connection to e.g., obtain
a dispatcher).

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
@zuercher
Copy link
Member Author

zuercher commented Aug 6, 2018

FYI, the first commit is just a revert of the revert commit (6a1868d). The second commit fixes the bug.

Signed-off-by: Stephan Zuercher <stephan@turbinelabs.io>
@zuercher
Copy link
Member Author

zuercher commented Aug 8, 2018

@mattklein123 or @danielhochman, since I blew up one of your deploys already, do you want to experiment with this change or shall I go ahead and merge it?

@mattklein123
Copy link
Member

Will defer to @danielhochman @ccaraman @junr03 if they want to smoke test. Might be worth it.

@zuercher zuercher merged commit 04616d6 into envoyproxy:master Aug 9, 2018
PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request Aug 30, 2018
…oyproxy#4067)"

This reverts commit 04616d6.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request Aug 30, 2018
…yproxy#4067)"

This reverts commit 04616d6.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
PiotrSikora added a commit to istio/envoy that referenced this pull request Aug 30, 2018
…yproxy#4067)"

This reverts commit 04616d6.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
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