health check: add TCP "connect only" health check#924
Conversation
By specifying an empty array for "send" and "receive" bytes, Envoy will enter a health check mode where it considers a connection event to be a success. A new connection is created for each health check cycle. Fixes #336
|
@lyft/network-team |
| // sure what situations cause this. If this turns into a problem, we may need to introduce a | ||
| // timer and see if the connection stays alive for some period of time while waiting to read. | ||
| // (Though we may never get a FIN and won't know until if/when we try to write). In short, this | ||
| // may need get more complicated but we can start here. |
There was a problem hiding this comment.
What if you try to read from the socket? It should return EAGAIN in case there is nothing to read (success) or 0 if connection was closed.
There was a problem hiding this comment.
That's a good idea, but not easily doable with the current connection interface. Let me add a TODO with that idea.
|
|
||
| setupNoData(); | ||
| cluster_->hosts_ = {HostSharedPtr{new HostImpl( | ||
| cluster_->info_, "", Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, "")}}; |
There was a problem hiding this comment.
@hennna might have some insight into how things like this should work to support IPv6 only test environments.
There was a problem hiding this comment.
I think this should be OK since it doesn't actually create a socket, but not sure. In general though would rather not deal with that in this change since the rest of this file looks the same and if there are changes required she can fix them all at the same time.
| // sure what situations cause this. If this turns into a problem, we may need to introduce a | ||
| // timer and see if the connection stays alive for some period of time while waiting to read. | ||
| // (Though we may never get a FIN and won't know until if/when we try to write). In short, this | ||
| // may need get more complicated but we can start here. |
There was a problem hiding this comment.
nit: ... may need to get ...
| parent_.dispatcher_.deferredDelete(std::move(client_)); | ||
| } | ||
|
|
||
| if ((events & Network::ConnectionEvent::Connected) && parent_.receive_bytes_.empty()) { |
There was a problem hiding this comment.
Per your description above, should we also check that send_bytes is empty here? (Not sure if we do, and/or want to support "one-way" modes, relying on transmission acks.)
There was a problem hiding this comment.
Let me add a comment/TODO. There is no way with the current interfaces to actually make this work.
| Health checks that require a more complex pattern such as send/receive/send/receive are not | ||
| currently possible. | ||
|
|
||
| If both "send" and "receive" are empty arrays, Envoy will perform "connect only" TCP health |
There was a problem hiding this comment.
I would just clarify this line and remove the reference to the "send" array, since envoy enters this mode specifically when "receive" is empty.
|
@goaway updated |
This reverts commit 1a7b0f9.
Automatic merge from submit-queue. [DO NOT MERGE] Auto PR to update dependencies of proxy This PR will be merged automatically once checks are successful. ```release-note none ```
Create a new thread for response handlers. The previous implementation uses whatever thread that envoy used for executing responses. Signed-off-by: Alan Chiu <achiu@lyft.com> For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md) Description: android: use new thread for responses handlers Risk Level: low Testing: ci Docs Changes: n/a Release Notes: n/a [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard <jp@jpsim.com>
Create a new thread for response handlers. The previous implementation uses whatever thread that envoy used for executing responses. Signed-off-by: Alan Chiu <achiu@lyft.com> For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md) Description: android: use new thread for responses handlers Risk Level: low Testing: ci Docs Changes: n/a Release Notes: n/a [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard <jp@jpsim.com>
5 participants
By specifying an empty array for "send" and "receive" bytes, Envoy
will enter a health check mode where it considers a connection event
to be a success. A new connection is created for each health check
cycle.
Fixes #336