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

x/net/http2: add per-write timeouts (WriteByteTimeout) #61777

Closed
Tracked by #67810
neild opened this issue Aug 5, 2023 · 8 comments
Closed
Tracked by #67810

x/net/http2: add per-write timeouts (WriteByteTimeout) #61777

neild opened this issue Aug 5, 2023 · 8 comments

Comments

@neild
Copy link
Contributor

neild commented Aug 5, 2023

This issue is part of a project to move x/net/http2 into std: #67810

This specific proposal makes the HTTP/2 client and server configurations more consistent, which will simplify adding these configurations to net/http. (Also, it seems like a reasonable feature considered on its own.)


HTTP/2 transports have a Transport.WriteByteTimeout configuration setting, which sets the maximum time a single write to a connection may take. The timeout begins when a write is made, and is extended whenever any data is written.

This setting can be used to detect unresponsive connections when the timeout for an entire request may be large or unbounded.

I propose extending this feature to apply to HTTP/2 server connections.

package http2

type Server struct { // contains unchanged fields
	// WriteByteTimeout is the timeout after which a connection will be
	// closed if no data can be written to it. The timeout begins when data is
	// available to write, and is extended whenever any bytes are written.
	WriteByteTimeout time.Duration
}

I'd originally thought to propose adding WriteByteTimeout to HTTP/1 connections as well, but there's less motivation for it there: HTTP/1 connections only handle a single request at at time, and a connection can only be reused after a complete request/response cycle is completed. There's less need to separate the response write timeout from an individual byte-write timeout. In addition, it's pretty much impossible to implement a byte-write timeout when using the sendfile path.

Maybe it would make sense to add something like this for HTTP/1, but it's simpler to consider HTTP/2 for now.

@neild neild added the Proposal label Aug 5, 2023
@gopherbot gopherbot added this to the Proposal milestone Aug 5, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/516200 mentions this issue: http2: add Server.WriteByteTimeout

@AlexanderYastrebov
Copy link
Contributor

I'd originally thought to propose adding WriteByteTimeout to HTTP/1 connections as well

There is #61568 for HTTP/1.

@neild
Copy link
Contributor Author

neild commented Aug 7, 2023

There is #61568 for HTTP/1.

Thanks, I missed that one. Will comment there.

@rsc
Copy link
Contributor

rsc commented Jun 12, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jun 24, 2024

Have all remaining concerns about this proposal been addressed?

The proposal is to add one new field to http2.Server:

	// WriteByteTimeout is the timeout after which a connection will be
	// closed if no data can be written to it. The timeout begins when data is
	// available to write, and is extended whenever any bytes are written.
	WriteByteTimeout time.Duration

@rsc
Copy link
Contributor

rsc commented Jun 27, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to add one new field to http2.Server:

	// WriteByteTimeout is the timeout after which a connection will be
	// closed if no data can be written to it. The timeout begins when data is
	// available to write, and is extended whenever any bytes are written.
	WriteByteTimeout time.Duration

@rsc
Copy link
Contributor

rsc commented Jul 25, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is to add one new field to http2.Server:

	// WriteByteTimeout is the timeout after which a connection will be
	// closed if no data can be written to it. The timeout begins when data is
	// available to write, and is extended whenever any bytes are written.
	WriteByteTimeout time.Duration

@rsc rsc changed the title proposal: x/net/http2: add per-write timeouts (WriteByteTimeout) x/net/http2: add per-write timeouts (WriteByteTimeout) Jul 25, 2024
@rsc rsc modified the milestones: Proposal, Backlog Jul 25, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/601496 mentions this issue: http2: add Server.WriteByteTimeout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

4 participants