Add backpressure information to tee, clone #16804
Conversation
Add information on what causes backpressure to ReadableStream.tee, Request.clone, Response.clone.
| a response from the server and stream it to the browser, but also stream it to the | ||
| ServiceWorker cache. Since a response body cannot be consumed more than once, you'd need | ||
| two copies to do this. | ||
|
|
||
| A teed stream will backpressure to the speed of the *faster* consumed `ReadableStream`, |
There was a problem hiding this comment.
Drive-by comment here.
Even if I know well how connections and messages do work (I was able to manually decode TCP messages back then), as a non-native reader, I find the term backpressure difficult to understand. I think we should find a better term, or at least succintly define it the first time we use it in a page.
There was a problem hiding this comment.
Fair point. I realize that I was using backpressure as a verb like in the akka-streams documentation e.g. Source.alsoTo. But in nodejs stream and streams spec, backpressure is only used as a noun. Also the growable internal queue and support for both pull sources and push sources when implementing an underlying source make backpressure more complicated.
For a ReadableStream in the streams spec, backpressure is specifically the condition that controller.desiredSize <= 0, which a push underlying source is expected to check periodically and pause calling controller.enqueue(chunk), and the controller will stop repeatedly calling pull(controller) to the underlying source which might be a pull source.
There was a problem hiding this comment.
@teoli2003 I have added a technical explanation of backpressure in ReadableStream.tee, and also I opened a PR to the streams spec so hopefully the spec will match my description whatwg/streams#1234.
I also added an informal description of backpressure to Response.clone.
There was a problem hiding this comment.
This is interesting. I hadn't got to my reading around Tees yet!
FYI Just to be a little bit pedantic because my head is in this at the moment for byte streams work ...
A push underlying source does use backpressure to change how it enqueues new data - it will always enqueue whatever data it gets. If it gets "backpressure" it is supposed to signal the thing that is providing the data (e.g. a socket) that it should pause or throttle supply. The difference is that not every supply has a mechanism for throttling or pausing.
As you say, for a pull source the controller manages the pull requests based on need (spec appears to say that it keeps requesting until bufffers are filled (desired size = 0) then stops calling it until empty and more data is requested. Though it looks like there are different strategies you can use there.
There was a problem hiding this comment.
PS Backpressure is actually not a bad term IMO, but as you say @teoli2003 it needs to be properly defined. I'd do that by linking to the section in the using streams guide (and make sure that is OK).
Backpressure is a bit filling a somewhat empty a pipe - at a certain rate the water starts pushing back up the pipe, and eventually you can't fill it any more.
Preview URLs
FlawsNone! 🎉 External URLsURL: No new external URLs URL:
URL: No new external URLs (this comment was updated 2022-06-16 16:38:31.204691) |
|
Do you want me to rebase and squash and add the PR number (#16804) to the commit message? |
|
It's fine like this! Thanks a lot! Congratulations upon your first merged commit here! Welcome aboard! 🎉 |
Add information on what causes backpressure to ReadableStream.tee, Request.clone, Response.clone.
Summary
Add warning that
ReadableStream.tee,Request.clone,Response.clonewill backpressure to the speed of the faster consumer, and unread data will be buffered at the slowerReadableStreamwithout limit and without backpressure.Motivation
I researched this issue when I studied the discrepancy between nodejs implementations such as node-fetch and minipass-fetch based on
Readable.pipe(which backpressures to the slower consumed stream to limit buffering) and browsers (which backpressure to the faster consumed stream and have unlimited buffering). I wrote my findings in node-fetch/node-fetch#1568Supporting details
The original sin was that the implementation of tee in the streams spec backpressured to the faster rather than the slower stream whatwg/streams#311, which contradicted the bounded-buffering suggestion in Stream requirements:. You must be able to pipe a stream to more than one writable stream. which said, “The tee stream can use a number of strategies to govern how the speed of its outputs affect the backpressure signals it gives off, but the simplest strategy is to pass aggregate backpressure signals directly up the chain, thus letting the speed of the slowest output determine the speed of the tee.”
The informative text in the streams spec is ambiguous; it suggests that tee is useful for “independent” consumers (Streams Specification: 2.1. Readable Streams, Streams Specification 4.2.4. ReadableStream example with tee). But
tee()is dangerous to use for independent consumers, since the slower consumer buffers without backpressure or limit and will use up all your memory and crash. But if we can’t fixtee(), at least we should document its shortcomings..
Related issues
Metadata