Skip to content

Commit b453c04

Browse files
authored
errors: always teardown connections on errors (#939)
Currently, the proxy tears down connections on unhandled errors, unless those errors are timeouts. We special-case errors which are timeouts and don't close connection in those cases. However, this can result in connections failing to be torn down when the host is unreachable. If the proxy continues trying to reconnect after a connection error, the connection error won't be what makes it to the errors layer --- instead, the request will eventually time out while reconnecting. This gets the request 503ed, but the connection remains open --- which, when a client is trying to talk to a stale endpoint, results in the connection entering a stuck state. This branch fixes this by removing the special-case logic for timeouts, and always tearing down connections. I validated this change using the repro in linkerd/linkerd2#5871 and confirmed that the issue is resolved after removing the special handling for timeouts. This fixes linkerd/linkerd2#5871.
1 parent ebf67d6 commit b453c04

File tree

1 file changed

+3
-15
lines changed

1 file changed

+3
-15
lines changed

linkerd/app/core/src/errors.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,9 @@ impl<RspB: Default + hyper::body::HttpBody> respond::Respond<http::Response<RspB
191191
}
192192

193193
// Gracefully teardown the server-side connection.
194-
if should_teardown_connection(&*error) {
195-
if let Some(c) = self.close.as_ref() {
196-
debug!("Closing server-side connection");
197-
c.close();
198-
}
194+
if let Some(c) = self.close.as_ref() {
195+
debug!("Closing server-side connection");
196+
c.close();
199197
}
200198

201199
if self.is_grpc {
@@ -223,16 +221,6 @@ impl<RspB: Default + hyper::body::HttpBody> respond::Respond<http::Response<RspB
223221
}
224222
}
225223

226-
fn should_teardown_connection(error: &(dyn std::error::Error + 'static)) -> bool {
227-
if error.is::<ResponseTimeout>() || error.is::<tower::timeout::error::Elapsed>() {
228-
false
229-
} else if let Some(e) = error.source() {
230-
should_teardown_connection(e)
231-
} else {
232-
true
233-
}
234-
}
235-
236224
fn http_status(error: &(dyn std::error::Error + 'static)) -> StatusCode {
237225
if let Some(HttpError { http, .. }) = error.downcast_ref::<HttpError>() {
238226
*http

0 commit comments

Comments
 (0)