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 support for SETTINGS_HEADER_TABLE_SIZE #56054

Closed
elindsey opened this issue Oct 5, 2022 · 6 comments
Closed

x/net/http2: add support for SETTINGS_HEADER_TABLE_SIZE #56054

elindsey opened this issue Oct 5, 2022 · 6 comments

Comments

@elindsey
Copy link

elindsey commented Oct 5, 2022

Proposal

Support HPACK dynamic table sizes other than the default, specifically:

  • Add ability to configure the max dynamic table size for HPACK encoders/decoders in both the http2 server and client
  • Add correct handling of sent/received SETTINGS_HEADER_TABLE_SIZE

Context

HTTP/2 uses HPACK for header compression. HPACK is a stateful compression algorithm where both the encoder and decoder reference a static table (defined in the RFC) and maintain a dynamic table. This dynamic table defaults to 4096 bytes and has header fields added/removed in FIFO order over the course of the connection lifetime.

SETTINGS_HEADER_TABLE_SIZE is an h2 setting used to adjust the size of the HPACK dynamic table. The decoder communicates the largest dynamic table size it's willing to support via SETTINGS_HEADER_TABLE_SIZE, then the encoder on the other end of the connection chooses what size it wants to use (up to the decoder's max) and communicates that back to the decoder via a dynamic header table size update instruction. Thus, there are two relevant client settings: the allowable size we want to advertise for our decoder, and the max size we want to use for our encoder.

Use Case

The main motivation is to allow increasing the dynamic table size for http2 connections that multiplex disparate requests, specifically in proxies and load balancers. The default size of 4096 works well for connections servicing a single client, but when multiple clients are bundled together on a reverse proxy's shared upstream connection and sharing the same decoder/encoder, the small table size and churning hurts the HPACK compression ratio, increases bytes on the wire and increases latency.

References

@gopherbot gopherbot added this to the Proposal milestone Oct 5, 2022
@neild
Copy link
Contributor

neild commented Oct 5, 2022

The specific API is in https://go.dev/cl/435899:

type Server struct {
	// MaxDecoderHeaderTableSize optionally specifies the http2
	// SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
	// informs the remote endpoint of the maximum size of the header compression
	// table used to decode header blocks, in octets. If zero, the default value
	// of 4096 is used.
	MaxDecoderHeaderTableSize uint32

	// MaxEncoderHeaderTableSize optionally specifies an upper limit for the
	// header compression table used for encoding request headers. Received
	// SETTINGS_HEADER_TABLE_SIZE settings are capped at this limit. If zero,
	// the default value of 4096 is used.
	MaxEncoderHeaderTableSize uint32
}

type Transport struct {
	// MaxDecoderHeaderTableSize optionally specifies the http2
	// SETTINGS_HEADER_TABLE_SIZE to send in the initial settings frame. It
	// informs the remote endpoint of the maximum size of the header compression
	// table used to decode header blocks, in octets. If zero, the default value
	// of 4096 is used.
	MaxDecoderHeaderTableSize uint32

	// MaxEncoderHeaderTableSize optionally specifies an upper limit for the
	// header compression table used for encoding request headers. Received
	// SETTINGS_HEADER_TABLE_SIZE settings are capped at this limit. If zero,
	// the default value of 4096 is used.
	MaxEncoderHeaderTableSize uint32
}

This seems fine to me.

@ianlancetaylor
Copy link
Contributor

CC @neild @bradfitz

@rsc
Copy link
Contributor

rsc commented Oct 12, 2022

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 Oct 26, 2022

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

@rsc
Copy link
Contributor

rsc commented Nov 2, 2022

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

@rsc rsc changed the title proposal: x/net/http2: add support for SETTINGS_HEADER_TABLE_SIZE x/net/http2: add support for SETTINGS_HEADER_TABLE_SIZE Nov 2, 2022
@rsc rsc modified the milestones: Proposal, Backlog Nov 2, 2022
@gopherbot
Copy link

Change https://go.dev/cl/435899 mentions this issue: http2: add SETTINGS_HEADER_TABLE_SIZE support

WeiminShang added a commit to WeiminShang/net that referenced this issue Nov 16, 2022
Add support for handling of SETTINGS_HEADER_TABLESIZE in SETTINGS
frames.

Add http2.Transport.MaxDecoderHeaderTableSize to set the advertised
table size for new client connections. Add
http2.Transport.MaxEncoderHeaderTableSize to cap the accepted size for
new client connections.

Add http2.Server.MaxDecoderHeaderTableSize and MaxEncoderHeaderTableSize
to do the same on the server.

Fixes golang/go#29356
Fixes golang/go#56054

Change-Id: I16ae0f84b8527dc1e09dfce081e9f408fd514513
Reviewed-on: https://go-review.googlesource.com/c/net/+/435899
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
@dmitshur dmitshur modified the milestones: Backlog, Unreleased Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants