Inconsistency between [[push]]'s return value and [[onPull]] invocation in ReadableStream

[tyoshino]

ReadableStream.[[push]] returns true as far as the [[strategy]] permits.

However, BaseReadableStream.read() calls [[callPull]] to pull new data from the data source only when [[buffer]] is empty. It's Base* class level criteria for pull. So, in case where "[[buffer]] does not become empty but [[bufferSize]] goes down below [[strategy]]'s high water mark", the data source won't be notified of pull.

These two behaviors are inconsistent.

Maybe, there we should define a smaller method which is expected to return whether to pull in the base class, and put something like "return [[buffer]].empty()" in the base class and override it with the [[strategy]] based code in the ReadableStream.

[domenic]

I think this is consistent. The model is that you wait for the buffer to be drained entirely before re-starting the pull from the underlying source. The high-water mark is not the same as the low-water mark (see backpressure section of requirements).

I am not opposed to allowing more customizability to the strategy though, and your last paragraph does sound like a good way to do that. In general, the strategies right now are only geared toward implementing high-water marks, and do not allow implementing low-water marks. I did this because in Node they used to have low-water marks, but threw them out because they worked very poorly, and found that a low-water mark of zero was always best (from my understanding). Then again, @isaacs has suggested that there shouldn't even be a high water mark, i.e. that having one in Node streams is a mistake (#13).

I would really like his perspective on water marks in general. But without that my tendency would be:

• If possible, figure out a natural way for users to build in transforms that do high- or low-water marks (Can we add buffering strategies via a transform stream? #24). It isn't entirely obvious how to do this though.
• If that's not possible, then we should give them the entire toolbox and allow strategies to implement both high- and low-water marks.

I will try to grab his perspective sometime soon :)

[tyoshino]

OK. I understood. Thanks for detailed explanation.

Like TCP silly window syndrome demonstrated in long time ago, I agree that always pulling data on non-zero room in buffer is not the right thing to do.

In the experiences Node had, high latency data source was involved? I.e. large delay between pull and delivery of data. I'd like to understand if the experience applies to almost everything.

I will try to grab his perspective sometime soon :)

OK. Thanks

[tyoshino]

So, for now, can we have some comment to explain that ReadableStream is designed so that even when needsMoreData returns true, we don't pull until the buffer becomes empty?

[domenic]

In the experiences Node had, high latency data source was involved? I.e. large delay between pull and delivery of data. I'd like to understand if the experience applies to almost everything.

I can only relay secondhand, but one thing @isaacs keeps stressing is that Node has a pretty good set of testing grounds for all scenarios, due to its use of streams for a variety of situations, including TCP, UDP, TLS, GZIP, and Crypto, with both fast and slow sources and sinks. All of those stress the systems in different ways. So I am hopeful Node's experience will port over pretty well to all situations.

[domenic]

So, for now, can we have some comment to explain that ReadableStream is designed so that even when needsMoreData returns true, we don't pull until the buffer becomes empty?

Yeah, this is a good idea.

I am trying to revise the prose (make it shorter, and focus on the important things) in the official-lookin branch; see also #62. I don't think I've been able to cover this point yet but let's be sure to include it. In the meantime we can certainly add something to the readme in the place you think is most appropriate.

[domenic]

I am going to close this in favor of the HWM discussions taking place in #13, since in fact it looks like draining the buffer entirely is not the right approach... Let me know if you think there's something we could do short-term in relation to this though.