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: configurable server pings #67812

Closed
Tracked by #67810
neild opened this issue Jun 4, 2024 · 6 comments
Closed
Tracked by #67810

x/net/http2: configurable server pings #67812

neild opened this issue Jun 4, 2024 · 6 comments

Comments

@neild
Copy link
Contributor

neild commented Jun 4, 2024

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

A HTTP/2 client or server may send PING frames to its peer. (RFC 9113 Section 6.7)

The http2.ReadIdleTimeout and http2.PingTimeout fields configure an HTTP/2 client to send a PING when a connection has been idle for some amount of time, and to close the connection if no response is received.

The http2.Server does not support sending PINGs on idle connections.

I propose adding the ability to configure servers to send pings as well. This setting will be off by default.

package http2

type Server struct { // contains unchanged fields
	// ReadIdleTimeout is the timeout after which a health check using a ping
	// frame will be carried out if no frame is received on a connection.
	// If zero, no health check is performed.
	ReadIdleTimeout time.Duration

	// PingTimeout is the timeout after which a connection will be closed
	// if a response to a ping is not received.
	// If zero, a default of 15 seconds is used.
	PingTimeout time.Duration
}

Adding this feature removes an inconsistency between HTTP/2 client and server configurations. Aligning the two will make it easier to add support for configuring HTTP/2 features to net/http, since we can define a single configuration struct rather than separate ones for clients and servers.

@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 rsc moved this from Incoming to Active in Proposals Jun 12, 2024
@rsc
Copy link
Contributor

rsc commented Jun 24, 2024

Have all remaining concerns about this proposal been addressed?

The proposal is to add two new fields to http2.Server:

	// ReadIdleTimeout is the timeout after which a health check using a ping
	// frame will be carried out if no frame is received on a connection.
	// If zero, no health check is performed.
	ReadIdleTimeout time.Duration

	// PingTimeout is the timeout after which a connection will be closed
	// if a response to a ping is not received.
	// If zero, a default of 15 seconds is used.
	PingTimeout 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 two new fields to http2.Server:

	// ReadIdleTimeout is the timeout after which a health check using a ping
	// frame will be carried out if no frame is received on a connection.
	// If zero, no health check is performed.
	ReadIdleTimeout time.Duration

	// PingTimeout is the timeout after which a connection will be closed
	// if a response to a ping is not received.
	// If zero, a default of 15 seconds is used.
	PingTimeout time.Duration

@rsc rsc moved this from Active to Likely Accept in Proposals Jun 27, 2024
@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 two new fields to http2.Server:

	// ReadIdleTimeout is the timeout after which a health check using a ping
	// frame will be carried out if no frame is received on a connection.
	// If zero, no health check is performed.
	ReadIdleTimeout time.Duration

	// PingTimeout is the timeout after which a connection will be closed
	// if a response to a ping is not received.
	// If zero, a default of 15 seconds is used.
	PingTimeout time.Duration

@rsc rsc changed the title proposal: x/net/http2: configurable server pings x/net/http2: configurable server pings Jul 25, 2024
@rsc rsc moved this from Likely Accept to Accepted in Proposals Jul 25, 2024
@rsc rsc modified the milestones: Proposal, Backlog Jul 25, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/601497 mentions this issue: http2: add support for server-originated pings

@aojea
Copy link
Contributor

aojea commented Sep 16, 2024

This is really a nice addition that will help to improve reliability of systems using http2

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

5 participants