test: fixing flake. #11507
test: fixing flake. #11507
Conversation
Signed-off-by: Alyssa Wilk <[email protected]>
| // Make sure the connection isn't closed during request upload. | ||
| config_helper_.addConfigModifier( | ||
| [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& | ||
| hcm) { hcm.mutable_delayed_close_timeout()->set_seconds(1000 * 1000); }); |
There was a problem hiding this comment.
Sorry how does configuring the delayed close timeout to a large value deflake this?
There was a problem hiding this comment.
The client sends a large request.
Some time during request upload, Envoy gives up buffering, and sends a local error response.
The test waits for the response to be complete. If Envoy has a short delay close, it is possible that the local response is sent (but not read) while the request is uploading, then the connection is closed, and continued upload produces a reset before the request is complete.
Ideally the test client would be reading as it's writing and get the complete request, but that's not guaranteed today.
There was a problem hiding this comment.
Ah OK I see. Do you mind expanding the comment?
There was a problem hiding this comment.
Done.
IMO the real solution to this both for Envoy upstream and Envoy mobile is to drain the connection on error in case of connection:close etc. Long ago when Andres tackled drain close we agreed we should fix on both ends but I don't think we ever got around to the other side of it.
Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Alyssa Wilk <[email protected]>
This is one of those cases where we actually want delay-close: we are doing a large upload, Enovy sends a response, and we want to make sure the socket stays open long enough the response is received rather than short delay-close causing a connection reset during upload Risk Level: n/a (test only) Testing: indeed Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <[email protected]>
This is one of those cases where we actually want delay-close: we are doing a large upload, Enovy sends a response, and we want to make sure the socket stays open long enough the response is received rather than short delay-close causing a connection reset during upload Risk Level: n/a (test only) Testing: indeed Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <[email protected]> Signed-off-by: yashwant121 <[email protected]>
This is one of those cases where we actually want delay-close: we are doing a large upload, Enovy sends a response, and we want to make sure the socket stays open long enough the response is received rather than short delay-close causing a connection reset during upload
Risk Level: n/a (test only)
Testing: indeed
Docs Changes: n/a
Release Notes: n/a