Feedback from Vorner

[yoshuawuyts]

Vorner wrote a really insightful post on trying out async/await with async-std, and talked about several things that didn't go well. This is a tracking issue to address all the feedback provided.

Also @vorner if you're reading along; thanks so much for the writeup! -- Posts like these are incredibly helpful, and allow us to learn where we need to improve.

Items

• Low rates of connection on localhost (started on in Implement simple work stealing #205)
• Investigate why TcpListener sometimes stays open indefinitely
• Investigate why connections are timing out
• Document how to enable the unstable feature (needed for future::timeout. (solved by Clean up the fs module and a few other places #190 Document feature flags in readme #202)
• futures_rs::select requires Unpin (solved by add future::{join,try_join,select,try_select} macros #187)
• Pinning can be tricky (groundwork in expose std::pin #203)

Quotes

But when running it, with both server and client on localhost, sometimes the experiment never finished, even with low rates of new connection creation. This seemed odd.

---

Still, even with rates like 100 new connections per second, some of the connections were timing out.

Code Example

// expected
async fn connect(server: SocketAddr, content: Arc<[u8]>, mut results: Sender<Duration>) {
    let res = timeout(connect_inner(server, content), TIMEOUT).await.unwrap();
    res.send().await.unwrap();
}

// current
async fn connect(server: SocketAddr, content: Arc<[u8]>, mut results: Sender<Duration>) {
    let connect = connect_inner(server, content);
    pin_mut!(connect);
    let timeout = task::sleep(TIMEOUT);
    pin_mut!(timeout);
    match future::select(connect, timeout).await {
        Either::Left((Ok(duration), _)) => results
            .send(duration)
            .await
            .expect("Channel prematurely closed"),
        Either::Left((Err(e), _)) => error!("Connection failed: {}", e),
        Either::Right(_) => {
            warn!("Connection timed out");
            results
                .send(TIMEOUT)
                .await
                .expect("Channel prematurely closed");
        }
    }
}

Refs

• https://github.com/vorner/tspam
• https://vorner.github.io/2019/09/15/play-with-new-async.html

[vorner]

Hello. It's nice you liked it.

Anyway, let me clarify a bit:

• I didn't complain about low rates on localhost. I set the rate at which I want it to run, I was merely pointing out that even if I set a low rate, the connections were timing out (or getting stuck). My point there was it probably isn't caused by overloading something (eg. packets getting lost).
• I don't know anything about TcpListener being open indefinitely. Is it something you deduced from what I wrote?
• The connections timing out, I suspect they time out on the client side. I believe I switched to tokio on the client, but had the server still running with the async-std code and it worked. But I'm not 100% sure, maybe I only thought I have been keeping the async-std server around.

[ghost]

What is the outstanding work we need to do on this issue? If there is none, can we close?

[yoshuawuyts]

@stjepang Yeah, I think it's okay to close. The only part that could potentially still be an issue is:

The connections timing out, I suspect they time out on the client side.

But we haven't had any other reports, so it may indeed just have been the client side here.

---

@vorner thanks so much again for the article you wrote. Apologies I dropped off of this feedback issue, but the points you've raised have been incredibly useful to improve the flow of people discovering async-std for the first time!