Consider moving away from asyncio's flow control helpers

[aaugustin]

In #16 @RemiCardona says:

I've also been reading njs's "post async/await" post and the more I think about it, a module like websocket (and many others, really) could probably do away with all the whole transports._FlowControlMixin: writes could/should be coroutines that "block" until frame data has been flushed to kernel/libc buffers. The userspace/asyncio buffering is counterproductive. The drain fail being just the tip of the iceberg. But that's work for another PR.

Here's the history of the current design and the state of my thoughts on this question.

websockets was developed in parallel with asyncio, which was still called tulip back then. I started using the StreamReader abstraction because I needed to read lines (for HTTP headers in the handshake) and exact number of bytes (for websockets frames).

I remember that was quite inconvenient at the time; thankfully the APIs improved a lot since then.

Then significant changes happened in tulip to implement flow control and I adapted websockets to follow them. Not having to rewrite the whole protocol class, whose state isn't trivial, was a decisive factor in my design choices.

I ended up relying on private APIs, feeling bad about it, and thinking I should revisit that part of the design — the very part you're questioning — at some point in the future.

Fortunately further changes allowed me to reduce the dependency on private APIs to one private attribute in a parent class which I don't expect to disappear. At that point I no longer considered it useful to redesign the protocol class.

Honestly I was still learning about backpressure, why it's important in distributed systems, and how to implement it. I was aiming at convincing myself that I was using asyncio's flow control properly and that, if writes were too fast, a websockets app would slow down rather than fill up the server's memory and crash. I wasn't capable of evaluating whether asyncio's helpers were the best for that use cases; they just seemed to do the job.

I'm not fundamentally opposed to getting rid of them, but:

• I'd like to make sure it doesn't degrade performance in high-traffic scenarios (not your use case) where writing a bunch of frames in every system call improves efficiency — I believe that's the main argument for userspace buffering
• I'm afraid a very significant part of the test suite will need rewriting as it's very heavily coupled to the implementation of readers and writers, in order to check what happens when series of events happen in a specific order

For low-traffic, poor-bandwidth use cases, where server-side efficiency doesn't matter, setting the low and high water marks to zero may be the right answer.

[aaugustin]

I re-read https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/ — I had discussed it with the author when it was first published (and had them mention that websockets actually does what they advocate).

Backpressure

It doesn't make a compelling argument for moving away from asyncio's flow control, now that we made the effort to use it correctly. It does make a good argument for curio being less foot-gunny for a greenfield project.

Read-side buffering

We could hint at read-side buffering issues in the documentation. Obviously running with full buffers is not an acceptable equilibrium for a websocket server. If you find yourself permanently in this situation, you don't have enough capacity.

websockets itself has an internal read buffer, bounded to max_queue=32 by default. Backpressure is propagated by yield from self.messages.put(msg). This one needs to be reduced as well to eliminate bufferbloat.

Closing time

This delves more generally into the problem of whether an application level write buffer is useful.

Note that it ignores (or predates) yield from dest_write.wait_closed(), which is the proper solution here.

[aaugustin]

I'm not sure I want to get into a discussion of buffering and water marks in the docs... These are important, but not trivial concepts, and they're most likely better explained somewhere else.