Skip to content

Commit

Permalink
fix(trace): remove GotConn delay
Browse files Browse the repository at this point in the history
The delay causes a race condition in the go transport that results in a
502 Bad Gateway with:
  `endpoint_failure (readLoopPeekFailLocked: %!w(<nil>))`.

This happens because the transport peeks the first few bytes on the
connection and gets some data even though it doesn't expect any. This
causes it to go into an error state even though there is no error
resulting in the formatting directive to break.

This commit removes the delay and adds a note why we can't do this for
now. This will reduce the amount of requests we can retry because the
client will send data before we know that the connection is good. After
we sent _some_ data we can't be sure that the server hasn't started
processing, hence no retry in such cases.

See: https://cloudfoundry.slack.com/archives/C033ALST37V/p1680888356483179
See: golang/go#31259
Resolves: cloudfoundry/routing-release#316
  • Loading branch information
maxmoehl committed Apr 13, 2023
1 parent 93ddb36 commit b589fb7
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions proxy/round_tripper/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,14 @@ func traceRequest(req *http.Request) (*http.Request, *requestTracer) {
GotConn: func(info httptrace.GotConnInfo) {
t.gotConn.Store(true)
t.connInfo.Store(&info)
if !info.Reused {
// FIXME: workaround for https://github.com/golang/go/issues/59310
// This gives net/http: Transport.persistConn.readLoop the time possibly mark the connection
// as broken before roundtrip starts.
time.Sleep(500 * time.Microsecond)
}
// FIXME: due to https://github.com/golang/go/issues/31259 this breaks our acceptance tests and is dangerous
// disabled for now even though this will reduce the number of requests we can retry
// if !info.Reused {
// // FIXME: workaround for https://github.com/golang/go/issues/59310
// // This gives net/http: Transport.persistConn.readLoop the time possibly mark the connection
// // as broken before roundtrip starts.
// time.Sleep(500 * time.Microsecond)
// }
},
DNSStart: func(_ httptrace.DNSStartInfo) {
t.tDNSStart.Store(time.Now().UnixNano())
Expand Down

0 comments on commit b589fb7

Please sign in to comment.