pjfanning opened a new pull request, #1265:
URL: https://github.com/apache/pekko-http/pull/1265
### 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:
```scala
case "reset" => Some("RstStreamFrame")
...
case _ => None
```
and had no alias for DATA, so DATA frames were unthrottleable by
configuration.
Adding a plain `"data"` alias would not have helped. Throttling *every* DATA
frame at the configured rate throttles legitimate throughput along with it, so
nobody could realistically enable it for this purpose — which is presumably why
the config documents the throttle as being for "non-data frame types". Only the
empty frames need it, because data-carrying ones are already bounded by flow
control.
### Modification
Add an `"empty-data"` alias that charges DATA frames whose payload is empty.
It resolves to a name that is deliberately not a real `frameTypeName`, which
`frameCost` recognises and applies to empty DATA frames only:
```scala
case d: DataFrame if d.payload.isEmpty &&
frameTypesForThrottle.contains(EmptyDataFrameThrottleName) => 1
```
Off by default, like every throttle target other than `"reset"`.
### Result
An operator can throttle a flood of empty DATA frames without throttling
data-carrying ones.
### Tests
- `sbt "http-core/testOnly
org.apache.pekko.http.impl.engine.http2.Http2BlueprintSpec"` — pass (11 tests),
with a new case for the alias.
- `sbt "http2-tests/testOnly ...Http2ServerEmptyDataThrottleSpec
...Http2ServerEnableFrameTypeThrottleSpec
...Http2ServerDisableFrameTypeThrottleSpec"` — pass (3 tests). The new spec
floods empty DATA frames with the throttle configured for them and requires the
connection to be torn down. Verified it fails when the alias resolves to
`None`, which is the behaviour before this change.
- `sbt http-core/mimaReportBinaryIssues` — pass.
- Header on the new file generated with `sbt headerCreateAll`; native
`scalafmt` clean.
### References
Refs #332 - extends the frame type throttle to empty DATA frames
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
