Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: make Transport retry more aggressively on failures if GetBody is set #17844

Closed
bradfitz opened this issue Nov 8, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@bradfitz
Copy link
Member

commented Nov 8, 2016

Make both the http1 and http2 Transports more aggressive on idempotent retries, if the new Go 1.8 Request.GetBody is defined. That means we can back up the Body and replay the whole request.

Go 1.8 uses that for redirects, but not for connection failures.

Do that in Go 1.9.

@kavirajk

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2016

can I work on this? @bradfitz

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Dec 14, 2016

Sure, if you're familiar with the http code. Note that we already did this for the http2 Transport (during server graceful shutdown in GOAWAY), so be sure to read that code first.

@kavirajk

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

@bradfitz Thanks. I will spend some time this weekend.

@kavirajk

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2017

Sorry for very late reply.

I started going through the code, to understand the problem correctly.

I noticed like you mentioned, http2 transport already creating the retry Request via Request.GetBody in http2shouldRetryRequest function inside h2_bundle.go

At the same time, in transport.go, the same operation is done via
pconn.shouldRetryRequest()
and
treq := &transportRequest{Request: req, trace: trace}

creating new request every time. Instead we have use to Request.GetBody to achieve that.

@bradfitz

@gopherbot

This comment has been minimized.

Copy link

commented Apr 28, 2017

CL https://golang.org/cl/42142 mentions this issue.

@glasser

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2017

To clarify: is this issue specifically about retrying on idempotent-method requests (GET/HEAD/OPTIONS/TRACE) which have non-nil Body but have GetBody set?

@gopherbot gopherbot closed this in eea8c88 Jun 5, 2017

@golang golang locked and limited conversation to collaborators Jun 5, 2018

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Dec 4, 2018

In Go 1.12 we now have the fix for #19943 (comment) ....

Change https://golang.org/cl/147457 mentions this issue: net/http: make Transport respect {X-,}Idempotency-Key header

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.