proposal: x/net/http2: add support for RFC 9218 priorities

[nicholashusin]

Proposal Details

In x/net/http2, http2.WriteScheduler is used to determine the order in which data is written to HTTP/2 streams.

x/net/http2 currently implements three write schedulers: round-robin (the default), random, and priority. The priority write scheduler is buggy, CPU hungry, and based off a prioritization scheme from RFC 7540 that has been deprecated as of RFC 9113 (see #67817). As part of moving x/net/http2 into std (#67810), we would ideally like to deprecate this write scheduler.

While most users are likely to use the default round-robin schedulers, users of the priority write scheduler does exist, which prevents us from deprecating it. Therefore, to move #67817 along, we want to add support for RFC 9218 priority, such that users can move off the old RFC 7540 priority scheduler.

I propose that we take the following actions:

1. Implement RFC 9218 priority write scheduler

To overly-simplify https://www.rfc-editor.org/rfc/rfc9218.html, we will create a new priority write scheduler that prioritizes streams based on priority provided by clients. Like the round-robin write scheduler, this scheduler and its constructor will not be exported. The priority provided by client is composed of two aspects:

• urgency: a number between 0 and 7, inclusive, where lower number denotes a higher urgency. Defaults to 3.
• incremental: a boolean, where false indicates that a stream can benefit from getting partial chunks, rather than needing to wait for the complete response. Defaults to false.

Our new scheduler can then make the following prioritization:

• It will prioritize streams with lower urgency value (i.e. highest urgency), before streams with higher urgency value.
• Given a group of incremental streams, it will round-robin writing between them, as they can all benefit immediately from partial responses. Conversely, for non-incremental streams, the scheduler will focus on writing to streams one-by-one till completion.
• If streams are coming from a proxy / intermediary, we override all received priority to urgency=3 and incremental=true to ensure fairness among end-users of the proxy.

At this stage, there are no API changes, and the write scheduler is not really usable end-to-end yet.

2. Implement RFC 9218 end-to-end

To allow the RFC 9218 prioritization to work end-to-end, we need to propagate the priority that clients provide (via headers and/or PRIORITY_UPDATE frame) to our write scheduler. Doing so involves the following steps and corresponding API changes:

Framer support for PRIORITY_UPDATE frame

We will add support for reading and writing RFC 9218 PRIORITY_UPDATE frames to http2.Framer. Strictly speaking, this does not need to be an exported API, but doing so is consistent with handling for other frames. Exported API changes are as follows:

package http2

const FramePriorityUpdate FrameType = 0x10

type PriorityUpdateFrame struct {
    FrameHeader
    Priority string
    PrioritizedStreamID uint32
}

func (f *Framer) WritePriorityUpdate(streamID uint32, priority string) error {} 

New SETTINGS_NO_RFC7540_PRIORITIES setting

RFC 9218 defines a SETTINGS_NO_RFC7540_PRIORITIES setting to permit clients or servers to indicate that they are not using the deprecated RFC 7540 prioritization scheme.

We will add a const for this setting, matching other settings handled by the http2 package:

package http2

const SettingNoRFC7540Priorities SettingID = 0x9

RFC 9218 does not provide a prioritization scheme negotiation mechanism. There is no way for a server to indicate that it supports RFC 9218 priorities, but can fall back to RFC 7540 when available.

When the HTTP/2 server uses a write scheduler that we know does not use RFC 7540 priorities (round robin or the new RFC 9218 scheduler), we will send a SETTINGS_NO_RFC7540_PRIORITIES value of 1 in its initial SETTINGS. Consequently, any PRIORITY frame that the server receives will be ignored.

3. Use the RFC 9218 priority write scheduler as the default

After RFC 9218 support is fully implemented, we can swap out the default scheduler that user gets from the round-robin scheduler to the RFC 9218 priority scheduler.

Worth noting here, is that there will be a significant behavior change if we follow the RFC 9218 recommendation strictly:

• The current default round-robin scheduler distributes writes evenly across all streams.
• With the new priority scheduler, it is likely that the majority of streams will be handled one-by-one until completion. This is because the default priority has incremental as false. Under the assumption that many HTTP/2 requests do not provide priority, most of the streams received will cause our scheduler to avoid round-robin.

To avoid a sudden significant behavior change for those who might be unaware of RFC 9218, we will only use false as the default incremental value for connections that has indicated that it is aware of RFC 9218 priorities. That is to say:

• When a client has never sent a priority header or PRIORITY_UPDATE frame, we will use urgency=3 and incremental=true as the default value for all of its streams.
• Once the client has sent a priority header or PRIORITY_UPDATE frame at least once, we will use urgency=3 and incremental=false as the default value.

This ensures that clients who do not utilize RFC 9218 will retain its previous behavior (urgency=3 and incremental=true for all streams are effectively round-robin).

Additionally, we will also add a new field within http.Server to allow users to disable stream prioritization if they so choose:

type Server struct {
  // Existing fields...
  DisableClientPriority bool // When true, round-robin across all streams.
}

After this is done, #67817 can then make further progress on deprecating the http2.WriteScheduler interface.

[gabyhelp]

Related Issues

• proposal: x/net/http2: deprecate WriteScheduler #67817
• net/http: switch default HTTP/2 write scheduler to round robin [freeze exception] #67706 (closed)
• net/http: use the http2 priority scheduler by default #18318 (closed)

Related Code Changes

• net/http: for http2, use the priority write scheduler by default

Related Discussions

• net/http: move HTTP/2 into the standard library

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[gopherbot]

Change https://go.dev/cl/704757 mentions this issue: http2: move out unexported symbols for RFC 7540 priority write scheduler

[gopherbot]

Change https://go.dev/cl/704758 mentions this issue: http2: introduce a new write scheduler based on RFC 9218 priority scheme

[gopherbot]

Change https://go.dev/cl/705917 mentions this issue: http2: add initial support for PRIORITY_UPDATE frame defined in RFC 9218

[gopherbot]

Change https://go.dev/cl/706115 mentions this issue: http2: add unexported function to parse RFC 9218 priority string

[gopherbot]

Change https://go.dev/cl/706755 mentions this issue: internal/httpsfv: add functionality to walk Parameters in HTTP SFV.

[aclements]

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

[gopherbot]

Change https://go.dev/cl/708015 mentions this issue: internal/httpsfv: add support for consuming Display String and Date type

[gopherbot]

Change https://go.dev/cl/708435 mentions this issue: internal/httpsfv: add parsing functionality for types defined in RFC 8941

[gopherbot]

Change https://go.dev/cl/708437 mentions this issue: internal/httpsfv: implement parsing support for date and display string

[aclements]

How does PriorityUpdateFrame relate to the existing PriorityFrame? If I understand correctly, PriorityFrame is part of the RFC 7540 scheme, but I didn't see it on the list of things to deprecate.

You used PriorityParam as part of PriorityUpdateFrame, which is already defined as part of PriortyFrame. PriorityParam uses the terms "exclusive" and "weight", which I think are concepts from RFC 7540. Do these relate to "urgency" and "incremental" from RFC 9218? Should this just be a new type instead of reusing PriorityParam?

[nicholashusin]

How does PriorityUpdateFrame relate to the existing PriorityFrame? If I understand correctly, PriorityFrame is part of the RFC 7540 scheme

That is correct yes. "PRIORITY" frame is part of RFC 7540, and "PRIORITY_UPDATE" frame is part of RFC 9218. They are distinctly different, despite their confusingly similar name.

but I didn't see it on the list of things to deprecate.

Hm sorry, just to clarify, by "deprecation", do we mean changing / breaking old behavior? Or the removal of the symbol entirely? I just realized that I've been using it to mean both in the original comment, apologies. Let me edit it for clarity so that "deprecation" strictly means the removal of a symbol.

We do want to change / break the old behavior of PriorityFrame: when the server receives a PriorityFrame, rather than adjusting the stream priority like before, it should instead send a SETTINGS frame with SETTINGS_NO_RFC7540_PRIORITIES set to 1.

However, we do not want to remove the PriorityFrame symbol, since we still need to be able to respond appropriately to clients who send PriorityFrame (i.e. sending SETTINGS_NO_RFC7540_PRIORITIES information).

You used PriorityParam as part of PriorityUpdateFrame, which is already defined as part of PriortyFrame. PriorityParam uses the terms "exclusive" and "weight", which I think are concepts from RFC 7540. Do these relate to "urgency" and "incremental" from RFC 9218? Should this just be a new type instead of reusing PriorityParam?

I think we originally reused PriorityParam so we do not add yet another exported symbol. I personally don't have a strong feeling between reusing PriorityParam or having a new type. I would be okay with either.

The RFC 7540 priority signals ("exclusive" and "weight") does not strictly relate to the RFC 9218 priority signals ("urgency" and "incremental). Although, at some point, @neild and I were discussing that it might be possible to use the RFC 7540 to approximate a reasonably good enough RFC 9218 signals. In which case, we might be able to support RFC 7540 priority signals still (we'd just internally translate it to the RFC 9218 priority signals, and use the RFC 9218 write scheduler). This should probably go beyond the scope of this proposal though.

[aclements]

Let me edit it for clarity so that "deprecation" strictly means the removal of a symbol.

We can't remove symbols that have already appeared in a released Go version. We can mark them deprecated, though, which is what we should do in this case.

I think we originally reused PriorityParam so we do not add yet another exported symbol. Having a new type sounds good to me.

Sounds good. Could you specify what that type would be, with documentation?

@neild pointed out that the priority parameter should actually be a string (expected to be encoded according to the RFC), so PriorityUpdateFrame should contain a string instead of PriorityParam and WritePriorityUpdate should take a string. (We discussed doing something more strongly-typed, but decided that we should keep this as simple as possible. In the future, we may have an API for working with Structured Field Values (SFVs), but that would likely also just operate on strings, keeping this weakly coupled.)

[nicholashusin]

We can't remove symbols that have already appeared in a released Go version. We can mark them deprecated, though, which is what we should do in this case.

Ah okay! I was under the impression that this is okay within /x/ libs, oops. Nevermind then. Just marking them as deprecated is good.

With regards to your original question on deprecating PriorityFrame, I think we do not want to mark it (and other things within Framer) as deprecated. Even if we stop supporting RFC 7540 priority, there are valid reasons for users to otherwise read / write frames if they're dealing with lower-level HTTP internals.

@neild pointed out that the priority parameter should actually be a string (expected to be encoded according to the RFC), so PriorityUpdateFrame should contain a string instead of PriorityParam and WritePriorityUpdate should take a string. (We discussed doing something more strongly-typed, but decided that we should keep this as simple as possible. In the future, we may have an API for working with Structured Field Values (SFVs), but that would likely also just operate on strings, keeping this weakly coupled.)

Oh indeed, the original comment is outdated. Edited it.

Sounds good. Could you specify what that type would be, with documentation?

Since we'd be using a Structured Field Values string as part of PriorityUpdateFrame, I don't think we'll need a new type anymore.

We'd still need to find a way to pass the Structured Field Values string to the write scheduler. This is currently done via http2.WriteScheduler's AdjustStream which we cannot change.

There are different ways to go about this:

• We keep using PriorityParam for RFC 9218's priority signal so we can use the existing AdjustStream. In this case we'd add urgency and incremental as an unexported field of PriorityParam.
• Within WriteScheduler interface, we create a new unexported method. Something like adjustStreamSFV(streamID uint32, priority string).
• We avoid using WriteScheduler interface entirely.

In any case though, this would be an implementation detail that does not add any new exported symbols, so I think we are good in this regard proposal-wise.

[nicholashusin]

After asking and looking around, I realized that me editing the original proposal iteratively as discussion moves along is rather unusual (seems like folks generally prefer to keep commenting new versions of the proposal?)

Apologies for that. Since the deed is already done, I'll keep on editing the original comment for this thread to avoid further confusion. Please consider the latest version of the original comment in this thread to be my proposal.

I'll avoid edits to the original comment in any future proposals I make.

[aclements]

func (f *Framer) WritePriorityUpdate(streamID uint32, p PriorityParam) error {}

I believe p should be a string? Let's also call the parameter priority.

type PriorityUpdateFrame struct { FrameHeader Priority string PrioritizedStream uint32 }

Let's rename PrioritizedStream to PrioritizedStreamID, since these frame types usually use the term "StreamID", and that's more consistent with the parameter to WritePriorityUpdate.

3. Deprecate the existing RFC 7540 priority write scheduler

Let's hold off on actually marking functions as deprecated for one release so people can transition to the new API. Though rewriting http2.NewPriorityWriteScheduler to return the new priority scheduler is fine.

[nicholashusin]

I believe p should be a string? Let's also call the parameter priority.

Oops, yes. Updated it.

Let's rename PrioritizedStream to PrioritizedStreamID, since these frame types usually use the term "StreamID", and that's more consistent with the parameter to WritePriorityUpdate.

Sounds good to me!

Let's hold off on actually marking functions as deprecated for one release so people can transition to the new API. Though rewriting http2.NewPriorityWriteScheduler to return the new priority scheduler is fine.

A newbie question on my end if you don't mind: without actually marking things as deprecated (which are then surfaced via tooling users use), how do we reliably notify users that the RFC 7540 write scheduler is being planned for deprecation and they need to start making the transition to the new API?

Is the concern here that users would suddenly have no option to use the old write scheduler, before they can fully transition to the new one?

If so, what if we do the following:

1. We make http2.NewPriorityWriteScheduler return the new RFC 9218 write scheduler only when provided with a nil *PriorityWriteSchedulerConfig. Users can still use the RFC 7540 write scheduler by providing an explicit config for it. At this point, we mark the RFC 7540 write scheduler symbols as deprecated, but in the deprecation info we mention that using the RFC 7540 write scheduler is still possible until the next release by providing an explicit config.
2. On the next release, we make http2.NewPriorityWriteScheduler only return the RFC 9218 write scheduler, and any config provided to it will be a no-op. The deprecation info is then updated to mention that using any of RFC 7540 write scheduler features are a no-op.

(Obligatory disclaimer that I'm saying this as someone new with no experience on deprecating stuff in our libraries. I'm mostly trying to understand the best practices that we usually follow, and I'm happy to just follow a standard deprecation procedure, if we have any.)

[aclements]

A newbie question on my end if you don't mind: without actually marking things as deprecated (which are then surfaced via tooling users use), how do we reliably notify users that the RFC 7540 write scheduler is being planned for deprecation and they need to start making the transition to the new API?

We would mention the planned deprecation in the release notes. In general, we like to give people a window where both the new thing and the old thing are fully supported. Admittedly, this is less critical for deprecation.

Do I understand correctly that connections will always the RFC 9218 scheduler by default after this change? Or do people have to call http2.NewPriorityWriteScheduler to get the RFC 9218 scheduler? If people will get it by default and http2.NewPriorityWriteScheduler will become simply unnecessary, then I think we should keep http2.NewPriorityWriteScheduler unchanged for one release, even will a nil argument, with the advice to just stop calling that if you want RFC 9218, and then mark it deprecated in the next release.

[nicholashusin]

We would mention the planned deprecation in the release notes. In general, we like to give people a window where both the new thing and the old thing are fully supported. Admittedly, this is less critical for deprecation.

Ack, thanks!

Do I understand correctly that connections will always the RFC 9218 scheduler by default after this change? Or do people have to call http2.NewPriorityWriteScheduler to get the RFC 9218 scheduler? If people will get it by default and http2.NewPriorityWriteScheduler will become simply unnecessary, then I think we should keep http2.NewPriorityWriteScheduler unchanged for one release, even will a nil argument, with the advice to just stop calling that if you want RFC 9218, and then mark it deprecated in the next release.

Eventually yes. The original plan is to do the following in steps:

1. Make http2.NewPriorityWriteScheduler return the RFC 9218 scheduler (and still keep the default scheduler as the current round-robin scheduler).
2. If things look good (no one using the RFC 9218 scheduler complains), change the default scheduler to the RFC 9218 scheduler too.

In https://go.dev/cl/704758, the RFC 9218 scheduler does perform reasonably close to the current default round-robin scheduler. So, I also would be fine if we were to directly change the default scheduler to the RFC 9218 scheduler, and leave http2.NewPriorityWriteScheduler unchanged.

[aclements]

Let's leave all of the deprecation as part of https://github.com/golang/go/issues/67817.

It seems like we should just change the default scheduler to RFC 9218 (with the tweak to the default priority parameters) and eliminate the round-robin scheduler. For the initial release with RFC 9218 support, let's leave http2.NewPriorityWriteScheduler returning the RFC 7540 scheduler. In a later release, we'll simultaneously deprecate http2.NewPriorityWriteScheduler, make it an effective no-op, and fully remove the RFC 7540 scheduler.

[nicholashusin]

Sounds good! Edited the proposal so deprecation is not in-scope. We will instead swap out the default scheduler to the RFC 9218 priority scheduler.

[gopherbot]

Change https://go.dev/cl/728401 mentions this issue: http2: buffer the most recently received PRIORITY_UPDATE frame

[gopherbot]

Change https://go.dev/cl/729120 mentions this issue: http2: add support for setting RFC 9218 priority via header field

[gopherbot]

Change https://go.dev/cl/729140 mentions this issue: http2: only make streams non-incremental if clients know of RFC 9218

[gopherbot]

Change https://go.dev/cl/729141 mentions this issue: http2: support SETTINGS_NO_RFC7540_PRIORITIES in SETTINGS frame