Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alan tries using a socket Sink #97

Merged
merged 21 commits into from
Apr 12, 2021
Merged
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
535807e
status quo: Carrie tries using a socket Sink
cryptoquick Mar 26, 2021
f2192c4
Switch to Alan.
cryptoquick Mar 26, 2021
fa2032f
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
83bdc33
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
d7c7489
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
2548488
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
2e85030
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
00073cb
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
9a41de3
Update src/vision/status_quo/carrie_tries_a_socket_sink.md
cryptoquick Mar 26, 2021
3f65592
Move the blog post into the story sources section.
cryptoquick Mar 26, 2021
dd7d632
Better phrasing, as suggested by @ibraheemdev.
cryptoquick Mar 26, 2021
15f2f7e
Rename file.
cryptoquick Mar 26, 2021
8686828
Apply suggestions from code review
cryptoquick Mar 31, 2021
abb5e7b
Add more specifics.
cryptoquick Mar 31, 2021
5e5cacc
Add more links and improve examples.
cryptoquick Mar 31, 2021
f27c77e
Add interop morals as per @eminence's suggestion
cryptoquick Mar 31, 2021
d97472e
Additional changes based on feedback.
cryptoquick Apr 5, 2021
2d51fe4
Add clarification of the task
cryptoquick Apr 5, 2021
2857f1a
Add rpc_ws_handler / WebSocketConnection function signature for context.
cryptoquick Apr 5, 2021
1b66a32
lol whoops
cryptoquick Apr 5, 2021
c741cce
Update src/vision/status_quo/alan_tries_a_socket_sink.md
nikomatsakis Apr 11, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/vision/status_quo/alan_tries_a_socket_sink.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ If you would like to expand on this story, or adjust the answers to the FAQ, fee

## The story

Alan is working on a project that uses an alternative popular async framework. He finds the new async framework less familiar, and as he reads through their GitHub issues, he's saddened by the project maintainer's unwelcoming attitude towards their community. He asks his teammates if they can switch to the one he's more familiar with and feels more welcome in, but the teammates are understandably resistant to changing something that important just for one feature. So, he works hard to find workarounds to accomplish his team's goals.
Alan is working on a project that uses async-std. He has worked a bit with tokio in the past and is more familiar with that, but he is interested to learn something how things work in async-std.

One of the goals is to switch from a WebSocket implementation using raw TCP sockets to one managed behind an HTTP server library, so both HTTP and WebSocket commands can be forwarded to a transport-agnostic RPC server. He finds an HTTP server that's similar to one he's used to using with the other async framework, and a WebSocket middleware library that goes with it.
One of the goals is to switch from a WebSocket implementation using raw TCP sockets to one managed behind an HTTP server library, so both HTTP and WebSocket commands can be forwarded to a transport-agnostic RPC server. He finds the HTTP server tide and it seems fairly similar to warp, which he was using with tokio. He also finds the WebSocket middleware library *foobar* that goes with it.
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved

However, as he's working, Alan encounters a situation where the socket needs to be written to within an async thread, and the traits just aren't working. He wants to split the stream into a sender and receiver:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having a hard time understanding this, actually, there seems to be some context I am missing. Poking at the rustdoc, I see the split method, that works on a Stream + Sink. Is the reason that this code doesn't work because ws_stream does not implement Sink? That could be made clearer -- for example, you could show the error message he gets.

Copy link
Contributor Author

@cryptoquick cryptoquick Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not really an error message so much as undesired locking behavior, and by that I mean, data can't be sent because there's a read lock. And an RwLock doesn't work in this scenario, because they're both iterables, which are mutable, and so everything would have to request a write lock.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this read lock? Inside the socket code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think maybe I'm starting to see. From reading the code snippet, what I get is this:

  • He wants to have incoming messages and then spawn off threads that will do some RPC before forwarding the messages on to another place?

I think what's confusing to me is that I think of a stream as an "async iterator", and I don't quite get how it could be split. It seems like what you want is to "splice" yourself in the middle of the stream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! I think async iterator is a good way to think of a stream as long as it's coupled with the perspective that new items can be added to the stream, I don't think it has to be finite. It's kinda like a duplex unbounded MPSC. But streams vary a great deal on their implementation details, such as high water mark, and backpressure signalling, but that's a little beyond the scope of this.

That said, I'm coming at this from a NodeJS perspective, where stream use is common, and often preferred. A WebSocket is an ideal candidate for this, but it's important it can be both read and written to in exclusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm totally for improvements to tide-websockets, but is that not outside the scope of this PR?

Copy link

@jbr jbr Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth figuring out if the Sink trait is specifically necessary, since that's what this issue and the closed issue were about. The issue that I believe is relevant to the async wg is whether Sink should be standardized, but if we can satisfy this use case without needing Sink, that seems worth attaching to this story.

The latter situation I described where any clone could act as sender or receiver wouldn't require an improvement, it's the current state of the code. The sender and receiver are locked internally, so there can only be one clone sending at a time, and one clone polling the stream at a time, but they can be sending and receiving concomitantly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the splicing was a misunderstanding on my part. I eventually realized that what was happening was just receiving messages, doing some processing, and responding in kind.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely changes to tide-websockets or whatever are not in scope for the story.

Copy link
Contributor

@nikomatsakis nikomatsakis Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the story itself should describe whether this can be solved without Sink, but I would definitely love to explore possible resolutions to this story as "shiny futures", or possibly as FAQs. I think we can merge without that. I talked to @cryptoquick some -- one thing that I was explaining is that it should be possible for him to add a newtype wrapped around the WebSocket type that implements futures::Sink and futures::Stream, which would then allow him to write his original example without having to maintain a complete parallel fork of the crate. This seems better to me.

It does seem to me that some trait that means "can consume values of type Item" would be appropriate. I am not convinced that the Sink trait as is the correct design for this (nor am I convinced of the opposite; just neutral).


Expand Down Expand Up @@ -67,19 +67,18 @@ while let Some(msg) = ws_stream.lock().await.next().await {

Alan wonders if he's thinking about it wrong, but the solution isn't as obvious as his earlier `Sink` approach. Looking around, he realizes a solution to his problems already exists-- as others have been in his shoes before-- within two other nearly-identical pull requests, but they were both closed by the project maintainers. He tries opening a third one with the same code, pointing to an example where it was actually found to be useful. To his joy, his original approach works with the code in the closed pull requests in his local copy! Alan's branch is able to compile for the first time.

However, almost immediately, his request was shut down like the others, and the maintainer suggests he try the complex, unobvious, and unspecific solution that he had already tried and just couldn't get it to work.
However, almost immediately, his request is closed with a comment suggesting that he try to create an intermediate polling task instead, much as he was trying before. Alan is feeling frustrated. "I already tried that approach," he thinks, "and it doesn't work!"

As a result of his frustration, Alan calls out one of the developers of the project on social media, who he believed had frequently shot down the idea of using `Sink` traits. This is not well-received, the maintainer reacts with a condescending and dismissive attitude, and he later finds out he was blocked. A co-maintainer responds to the thread, defending and supporting the other maintainer's actions, and he's told to "get over it". He's given a link to a blog post with a piece lamenting the popularity of Sink in the Rust ecosystem, and how it's complex, bad and not worth it. The piece unhelpfully makes no effort to provide examples of how the better alternatives would be used to replace uses of `Sink`.
As a result of his frustration, Alan calls out one developer of the project on social media. He knows this developer is opposed to the `Sink` traits. Alan's message is not well-received: the maintainer sends a short response and Alan feels dismissed. Alan later finds out he was blocked. A co-maintainer responds to the thread, defending and supporting the other maintainer's actions, and suggests that Alan "get over it". Alan is given a link to a blog post. The post provides a number of criticisms of `Sink` but, after reading it, Alan isn't sure what he should do instead.

Because of this heated exchange, Alan grows concerned for his own career, what these well-known community members might think or say about his to others, and his confidence in the community surrounding this language that he really enjoys using is somewhat shaken.

Despite this, Alan takes a walk, gathers his determination, and commits to maintaining his fork with the changes from the other pull requests that were shut down, and publishes his version to crates.io, vowing to be more welcoming to "misfit" pull requests like the one he needed.
Despite this, Alan takes a walk, gathers his determination, and commits to maintaining his fork with the changes from the other pull requests that were shut down. He publishes his version to crates.io, vowing to be more welcoming to "misfit" pull requests like the one he needed.

A few weeks later, Alan's work at his project at work is merged with his new forked crate. It's a big deal, his first professional open source contribution to a Rust project, but he still doesn't feel like he has a sense of closure with the community. Meanwhile, his friends say they want to try Rust, but they're worried about its async execution issues, and he doesn't know what else to say, other than to offer a sense of understanding. Maybe the situation will get better someday, he hopes.
A few weeks later, Alan's work at his project at work is merged with his new forked crate. It's a big deal, his first professional open source contribution to a Rust project! Still, he doesn't feel like he has a sense of closure with the community. Meanwhile, his friends say they want to try Rust, but they're worried about its async execution issues, and he doesn't know what else to say, other than to offer a sense of understanding. Maybe the situation will get better someday, he hopes.

## 🤔 Frequently Asked Questions

*Here are some standard FAQ to get you started. Feel free to add more!*

* **What are the morals of the story?**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole story is obviously about a real and specific event, which is great (we like specifics in these kind of stories).

But as I'm coming to the end of the story and thinking about morals, I'm also trying to draw generalizations too. Based on these comments:

but is frustrated to find that the Sink trait wasn't implemented in the WebSockets middleware library he's using.

lamenting the popularity of Sink in the Rust ecosystem, and how it's complex, bad and not worth it.

I'm wondering if there is also something to say here about the difficulties/friction in trying to get multiple async libraries to interop, especially when they have conflicting design philosophies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some more, do you think that helps?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly having core abstractions in stdlib would help here.

* There are often many sources of opinion in the community regarding futures and async, but these opinions aren't always backed up with examples of how it should be better accomplished. Sometimes we just find a thing that works and would prefer to stick with it, but others argue that some traits make implementations unnecessarily complex, and choose to leave it out. Disagreements like these in the ecosystem can be harmful to the reputation of the project and the participants.
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
Expand Down