Conversation
Motivation: A DATA frame carrying no payload has `sizeInWindow == 0`, so it consumes no flow-control window. The *number* of such frames is therefore not bounded by flow control at all, and on an open stream a peer can send them continuously - each one costing a pass through the stream state machine, a `buffer ++= empty`, and a flow-control recompute. The `frame-type-throttle` mechanism could not be pointed at them. It maps an alias to a frame type name and had no alias for DATA, so DATA frames were unthrottleable by configuration. A plain `"data"` alias would not have helped: throttling every DATA frame at the configured rate throttles legitimate throughput along with it, which is why the config documents the throttle as being for "non-data frame types". Not every empty DATA frame is suspect either. An empty DATA frame carrying END_STREAM is how a client closes a request body whose length it did not know up front - clients such as grpc and Go's net/http2 send one per such request - so it is both legitimate and self-limiting: one per stream, after which the stream is half-closed. At the default budget of 100 charged frames per second per connection, charging those would tear down connections carrying ordinary multiplexed traffic. An empty DATA frame that does not end its stream has no such use. Modification: Add two aliases: - `empty-data-no-end-stream` charges the empty DATA frames that do not carry END_STREAM. A peer has no reason to send those at all, so it joins `reset` in the default `frame-types`. - `empty-data` charges every empty DATA frame, END_STREAM included. Enabling it can throttle legitimate traffic, so it stays off by default. Both resolve to names that are deliberately not real `frameTypeName`s, which `frameCost` recognises. `frameCost` moves out of `rapidResetMitigation` into a `private[http2]` method so it can be tested directly; a frame that both aliases match is still charged once. Result: A flood of empty DATA frames that do not end their stream fails the connection out of the box, while the empty DATA frames that close a request body are left alone unless an operator opts in to `empty-data`. Tests: - sbt "http-core/testOnly org.apache.pekko.http.impl.engine.http2.Http2BlueprintSpec" - pass (17 tests); new cases for both aliases and for `frameCost` covering END_STREAM, data-carrying frames, frames matched by their own type name, and both aliases configured at once. - sbt "http2-tests/testOnly org.apache.pekko.http.impl.engine.http2.Http2ServerEmptyDataThrottleSpec org.apache.pekko.http.impl.engine.http2.Http2ServerEmptyDataNoEndStreamThrottleSpec org.apache.pekko.http.impl.engine.http2.Http2ServerEnableFrameTypeThrottleSpec org.apache.pekko.http.impl.engine.http2.Http2ServerDisableFrameTypeThrottleSpec" - pass (4 tests). The new Http2ServerEmptyDataNoEndStreamThrottleSpec sets no `frame-types` override, so it covers the default; verified it fails with the default put back to `["reset"]`. - sbt "http2-tests/testOnly org.apache.pekko.http.impl.engine.http2.Http2ServerSpec org.apache.pekko.http.impl.engine.http2.Http2ClientServerSpec org.apache.pekko.http.impl.engine.http2.Http2ClientSpec org.apache.pekko.http.impl.engine.http2.WithPriorKnowledgeSpec org.apache.pekko.http.impl.engine.http2.H2cUpgradeSpec" - pass (178 tests, 17 pending), checking the new default does not disturb ordinary traffic. - sbt http-core/mimaReportBinaryIssues - pass. - Header on the new file generated with `sbt http2-tests/headerCreateAll`; native `scalafmt` clean. References: Refs apache#332 - extends the frame type throttle to empty DATA frames
pjfanning
force-pushed
the
throttle-http2-data-frames
branch
from
September 10, 2026 10:24
1c34209 to
ce03d6e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
A DATA frame carrying no payload has
sizeInWindow == 0, so it consumes no flow-control window. The number of such frames is therefore not bounded by flow control at all, and on an open stream a peer can send them continuously — each one costing a pass through the stream state machine, abuffer ++= empty, and a flow-control recompute.The
frame-type-throttlemechanism could not be pointed at them. It maps an alias to a frame type name:and had no alias for DATA, so DATA frames were unthrottleable by configuration. A plain
"data"alias would not have helped: throttling every DATA frame at the configured rate throttles legitimate throughput along with it, which is presumably why the config documents the throttle as being for "non-data frame types".Not every empty DATA frame is suspect either. An empty DATA frame carrying END_STREAM is how a client closes a request body whose length it did not know up front — clients such as grpc and Go's
net/http2send one per such request — so it is both legitimate and self-limiting: one per stream, after which the stream is half-closed. At the default budget of 100 charged frames per second per connection, charging those would tear down connections carrying ordinary multiplexed traffic. An empty DATA frame that does not end its stream has no such use.Modification
Add two aliases, so the two cases can be configured separately:
empty-data-no-end-streamcharges empty DATA frames that do not carry END_STREAM. Nothing legitimate produces those, so it joinsresetin the defaultframe-types.empty-datacharges every empty DATA frame, END_STREAM included. Enabling it can throttle legitimate traffic, so it stays off by default.Both resolve to names that are deliberately not real
frameTypeNames, whichframeCostrecognises:frameCostmoves out ofrapidResetMitigationinto aprivate[http2]method so it can be tested directly. A frame that both aliases match is still charged once.The default becomes:
Result
A flood of empty DATA frames that do not end their stream fails the connection out of the box, while the empty DATA frames that close a request body are left alone unless an operator opts in to
empty-data.Tests
sbt "http-core/testOnly org.apache.pekko.http.impl.engine.http2.Http2BlueprintSpec"— pass (17 tests); new cases for both aliases and forframeCostcovering END_STREAM, data-carrying frames, frames matched by their own type name, and both aliases configured at once.sbt "http2-tests/testOnly ...Http2ServerEmptyDataThrottleSpec ...Http2ServerEmptyDataNoEndStreamThrottleSpec ...Http2ServerEnableFrameTypeThrottleSpec ...Http2ServerDisableFrameTypeThrottleSpec"— pass (4 tests). The newHttp2ServerEmptyDataNoEndStreamThrottleSpecsets noframe-typesoverride, so it covers the default; verified it fails with the default put back to["reset"].sbt "http2-tests/testOnly ...Http2ServerSpec ...Http2ClientServerSpec ...Http2ClientSpec ...WithPriorKnowledgeSpec ...H2cUpgradeSpec"— pass (178 tests, 17 pending), checking the new default does not disturb ordinary traffic.sbt http-core/mimaReportBinaryIssues— pass.sbt http2-tests/headerCreateAll; nativescalafmtclean.References
Refs #332 — extends the frame type throttle to empty DATA frames