Description
Summary
I don't think that poll_ready() provides enough value for it's extra complexity, and can be removed entirely.
Value of poll_ready
As I understand it (from Inventing the Service trait) poll_ready is a mechanism to provide back-pressure in the case that a service is not immediately available to handle a request. Each Poll::Ready(Ok(())) you receive gives you permission to send exactly one request in via call. Poll::Ready(Err(err)) indicates that this service will never be able to handle requests again.
This allows you to check if a service is able to handle a request before constructing it, which could be a more efficient if holding the request around is expensive. Note that it doesn't help if constructing the request is expensive because once you've called poll_ready you are committed to calling the service (unless the service has stopped but that's incredibly rare in my experience).
Issues with poll_ready
Difficult to adhere to the contract of poll_ready
Imagine a middleware service like Load Shed but with an additional queue to handle bursts of traffic. That service wants to: return ready from poll_ready when the queue is empty, return pending from poll_ready if the queue isn't full, and return ready again if the queue is full so that it can return an overload error from call. Similar surprisingly complex logic appears in services like Steer where it must keep track of which inner service it hasn't received a ready token from.
Not calling call after poll_ready
It's very easy to leak resources if you fail to call the Service and run that future to completion after receiving the go-ahead from poll_ready, see #408 for some existing discussion. Note that this is basically impossible to avoid at the moment, even if you write service.ready().await?.call(request).await?, the future might be dropped after ready has returned but before call has been polled. Additionally, if constructing the request is expensive, it probably can also fail (or is itself asynchronous), which would mean you have to build it before calling poll_ready!
I believe #412 would solve this issue, but at the same time adds more complexity to the API.
Implementing back-pressure without poll_ready
Fundamentally, I think that everything achieved by poll_ready can be achieved without it., and removing it would align much better with Rust's design philosophy of "make easy things easy and hard things possible"1.
Back-pressure
I am confused by the arguments in #3 and #112, so it's possible I'm missing something here, but moving the code from poll_ready to the start of (the future returned by) call in every Service would not change their behaviour. The only downside to doing this that I can see is that it would prevent you from delaying the construction of the request until the service is ready.
Delayed construction of requests
It's still possible to delay the construction of requests without poll_ready, using the MakeService pattern. You can implement a ReadyService: (()) -> Result<OneshotService, Error>, which gives you the ability to wait for something to be ready before constructing the request. It's easily composable with middleware on both the ReadyService and OneshotService side, and even gives you #412 for free because the OneshotService is, in essence, a token.
Conclusion
One slightly odd result of all this is that Service would become ~identical to FnMut(Request) -> F where F: Future<Output=Result<Response, Error>>, which makes me wonder slightly on what value it's providing. I'm interested to here what I've missed, why poll_ready was implemented in the first place, and whether this is a possible future (pun intended). Sorry this is so long but it's been sitting in my head for a while now and I needed to get it out.
Footnotes
-
There's probably a better source for this but: Rust API Guidelines - Type Safety ↩