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

Async IO #778

Merged
merged 2 commits into from
May 16, 2016
Merged

Async IO #778

merged 2 commits into from
May 16, 2016

Conversation

seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented May 4, 2016

This converts hyper to use non-blocking IO (or async). The control flow in the Server and Client APIs has breaking changes, but it was for the best.

The major pieces of an HTTP library are working here:

  • Server
  • Client
  • SSL
  • Keep-Alive (server only at the moment)
  • "Async" DNS (it's just a thread pool using getaddrinfo, but it won't block the loop!)

Docs can be previewed at here

Closes #395

@seanmonstar seanmonstar added this to the Async IO milestone May 4, 2016
@Ogeon
Copy link
Contributor

Ogeon commented May 4, 2016

It's very nice to see this move on to being a PR 🎉 I would just like to make another request for a way to destructure the Request in a server.

@tailhook
Copy link

tailhook commented May 4, 2016

Sorry if it's a wrong thread, but do you have any plans on async DNS in the long term?

I've started rotor-dns. It's very limited for now, but the major showstopper for it is not the DNS implementation, but all the plugins of gnu libc (i.e. you can configure /etc/nsswitch.conf and use ldap for getaddrinfo).

On the other hand, musl libc completely ignores all this stuff. And its a pretty uncommon to use anything other than DNS and /etc/hosts for resolving the names nowadays.

Any thoughts?

@seanmonstar
Copy link
Member Author

@Ogeon thanks for reminding me, I had forgotten. Just pushed up a deconstruct method on Request.

@tailhook I've been watching your rotor-dns. The idea excites me, I just wanted to wait for it to mature more. Another possibility is binding to c-ares, which has some pretty decent bindings, including an example of tying it into mio. I expect to open an issue to track improving DNS in hyper (interesting to note that other projects seem fine with a threadpool of getaddrinfo, such as libuv, h2o, ...)

@lambda
Copy link

lambda commented May 4, 2016

@tailhook Using mDNS is not uncommon, especially for home and small office networks using a lot of Mac clients. Apparently Windows may be getting better support for it soon, so it may become more widespread. So there's at least one other host resolution mechanism that's reasonably common. And apparently some people do use LDAP for name resolution as well, though I've never personally encountered it.

I think that if you're writing a server-side application that you fully control, just being able to use DNS plus /etc/hosts will usually be fine, but for deploying applications that other people might run in various different environments, you want to use your platform's name resolution, which generally means running getaddrinfo in a thread if you want it to be asynchronous. Having a async DNS resolver like rotor-dns is great, however, for situations in which it is sufficient.

@Ogeon
Copy link
Contributor

Ogeon commented May 4, 2016

@seanmonstar Thank you 😄 I think that was the last piece that was missing for me, in Hyper itself, apart from threading. That's another story, though.

@siddontang
Copy link

Hi @seanmonstar

We met an problem when using async DNS in another thread before.
If one host is down, getaddrinfo will take a long time (>1s in our test) to return failure, this may block following normal host resolving and then affect whole system.

We use rust-c-ares + mio now but it only works well in *inx, not windows.
I think maybe we can use c-ares + rotor later, but for windows, may still thread.

Btw, thanks your awesome async hyper, we are refactoring our network framework with it now. :-)

@seanmonstar
Copy link
Member Author

@siddontang yea, that is mitigated by increasing the amount of threads in the thread pool. That is a knob you can turn:

Client::configure()
    .connector(HttpConnector::default().threads(64))
    .build()

@Stebalien
Copy link
Contributor

This is a great feature and really exciting. However... I really hope you're planning on adding some convenience handlers/methods/blocking APIs to the client. While client API is very flexible and great for applications where performance is paramount, it's completely unusable for simple (usually single-threaded) applications (not hyperbole). I apologize for being so blunt but I feel that this needs to be said very clearly. These applications can always switch to the curl library but personally, I like avoiding C dependencies when unnecessary.

As for more actionable feedback, the client examples appear to be out of date.

@seanmonstar
Copy link
Member Author

@Stebalien Thanks for the feedback. I agree, use the Client api in this branch is harder, and some types of applications won't see much benefit for the work. (At least, they can now set connect timeouts.)

I decided a few months ago that hyper needs to be low level, and not making decisions that affect performance. There other applications who really need the performance gains. Especially servers that make client requests of their own. Think proxy or reverse proxy servers.

By leaving the building blocks there, I expect client libraries to appear that are more opinionated, catering to applications like rustup or cargo. They could chose to use Futures, callbacks, or anything else.

A synchronous API could even exist, exposing io::Read and Write. An example of that exists in the 'sync' example file.

What examples are out of date?

@Ogeon
Copy link
Contributor

Ogeon commented May 5, 2016

@seanmonstar I noticed that there's no try_clone for HttpsListener. Is this an oversight or by design?

@Stebalien
Copy link
Contributor

What examples are out of date?

The client example was, or, at least, I could have sworn it was (Client::new accepted a Handler).

By leaving the building blocks there, I expect client libraries to appear that are more opinionated, catering to applications like rustup or cargo. They could chose to use Futures, callbacks, or anything else.

A synchronous API could even exist, exposing io::Read and Write. An example of that exists in the 'sync' example file.

This is good and will allow hyper to integrate well with other libraries. However, releasing this as-is without some "hyper-sync" or "hyper-easy" library in place will either force users to abandon hyper, never upgrade, or do this work over-and-over. Basically, I'm objecting to the fact that this is a breaking change without a clear upgrade path.

@seanmonstar seanmonstar force-pushed the mio branch 3 times, most recently from 8b28288 to 2d47917 Compare May 6, 2016 17:13
@seanmonstar
Copy link
Member Author

@Ogeon by design. It doesn't actually make sense to require SSL implementations to make their context implement Clone. Instead, you could create a HttpListener, duplicate it a few times, and then construct SslContexts for them.

@Ogeon
Copy link
Contributor

Ogeon commented May 6, 2016

Alright, I think I got it working now, like this:

let listener = HttpListener::bind(host);

//... listener.try_clone(), etc.

let ssl = try!(Openssl::with_cert_and_key(cert, key));
let server = hyper::Server::new(HttpsListener::with_listener(listener.0, ssl));

@seanmonstar seanmonstar force-pushed the mio branch 9 times, most recently from b95b57f to c4810a2 Compare May 9, 2016 22:55
@teburd
Copy link
Contributor

teburd commented May 10, 2016

Out of curiosity, why is Handler still required to be Sync and Send now? It should be neither correct?

@seanmonstar
Copy link
Member Author

@BFrog server::Handler does not require Sync. It did have a Send bound, but more because I forgot to remove it. It wasn't needed. hyper still compiled as soon as I removed it.

@teburd
Copy link
Contributor

teburd commented May 11, 2016

It looks like HandlerFactory has a fn create which takes a http::Control, however... hyper::http is private, so its impossible currently to implement the trait outside of hyper I believe?

@seanmonstar
Copy link
Member Author

Control is reexported at the top level, hyper::Control.

On Wed, May 11, 2016, 8:41 AM Tom Burdick notifications@github.com wrote:

It looks like HandlerFactory has a fn create which takes a http::Control,
however... hyper::http is private, so its impossible currently to implement
the trait outside of hyper I believe?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#778 (comment)

@teburd
Copy link
Contributor

teburd commented May 11, 2016

So it is, apologies

@seanmonstar
Copy link
Member Author

The latest push just now changes the server from spawning its own thread, to allowing it to be executable on the current thread. This means that if you're creating multiple servers with the behavior that it spawned its own thread, you now need to move the loop into a thread and run it. It looks like this now:

let (listening, server) = Server::new(listener).handle(factory).unwrap();
thread::spawn(move || server.run());

However, this has removed the Send + 'static bounds on Handler, so you can now try to create Handlers with references.

@teburd
Copy link
Contributor

teburd commented May 13, 2016

@seanmonstar Fantastic! No more std::sync::mpsc errors :beers:

@Ogeon
Copy link
Contributor

Ogeon commented May 13, 2016

Then I suppose it should be possible to use scoped threads. Interesting...

@seanmonstar seanmonstar force-pushed the mio branch 2 times, most recently from 15a1988 to 06abb3e Compare May 13, 2016 22:19
@tailhook
Copy link

What do you think about KCM in linux 4.6? My thoughts are here: tailhook/rotor-stream#3 . In short: it's something which should be in "must have" category in a year.

@tailhook
Copy link

At the closer look, it looks like KCM is of very limited use for HTTP. So it's probably off-topic here.

BREAKING CHANGE: This breaks a lot of the Client and Server APIs.
  Check the documentation for how Handlers can be used for asynchronous
  events.
@seanmonstar
Copy link
Member Author

This feels fairly solid now, thanks to everyone having tested it. There is 1 failing issue on Windows, which seems to be that related to how mio implements IOCP. The specific issue is that the hyper Server drops the connection after the final write (and it's not marked keep-alive), but dropping it seems to abort the queued last write.

I'm going to merge this now, and hope the IOCP issue is looked into (if you develop on Windows, more debugging would certainly be useful!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants