feign
feign copied to clipboard
OpenFeign
Non-blocking I/O support
As per our discussion with @adriancole in #24 , it would be good to add non-blocking I/O support to Feign.
This can be particularly beneficial for the scenarios with API gateways or microservice API composition.
can someone in favor of this describe the api they wish to be non-blocking? or is it simply that they want the buffered http request that's invoked synchronously, to use a non-blocking library underneath?
First we want to use lib that use nio as underlying implementation.
in manner of async API I would like to pass to the feign a callback that will handle the response. in this case the API will return void.
such a proposal will end up with some severe api changes. Is anyone interested in working on this? I think there are several related issues where people ask for the same thing (which is what feign isn't designed for).
Since we have OpenFeign org now, I'm happy to create a repo for someone to experiment with this.
IOTW volunteer wanted!
What do you mean by "which is what feign isn't designed for"?
Feign's is not designed for asynchronous invocation or zero-copy i/o.
For example, requests are buffered up-front, and all i/o operations are blocking. Core concepts such as interceptors and exception handlers were not designed for asynchronous invocation. The Client interface (required by all http backends) is a synchronous api. Internal dispatch assumes synchronous apis, as well many try/catch blocks associated with it. RetryHandler assumes synchronous invocation
I could go on, but there are a lot of internals that are simpler because synchronous is assumed. When someone changes the invocation model, they must break api or certain things won't work. Also, depending on what's desired from nio, the contents of requests and responses may also need to be changed.
Hope this helps.
ps the associated issues are #24 #25 #220 as well as others that were closed dupe (as this probably should as it highly overlaps with #24)
Hi !
I understand Feign has not been designed with non blocking in mind. Maybe a new project is a better target. I would suggest using reactive stream as a standard way to handle IO events. First I am wondering if there is any proper reactive lightweight HTTP client out there.
probably want to collaborate on https://github.com/OpenFeign/feign-vertx for starters
On Sat, Sep 3, 2016 at 3:24 PM, Daniel Lavoie [email protected] wrote:
Hi !
I understand Feign has not been designed with non blocking in mind. Maybe a new project is a better target. I would suggest using reactive stream as a standard way to handle IO events. First I am wondering if there is any proper reactive lightweight HTTP client out there.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenFeign/feign/issues/361#issuecomment-244532121, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD61xREV5wavu_C1_M4X4DIEQp7-O2Vks5qmSDDgaJpZM4Hx9G3 .
Hello, I submitted a new feature to support java8 streams, it will allow to handle response body asynchronously or without waiting for the full body before to process.
Although request support IO is still blocking, response input stream handling can take benefits of java8 streams. Please vote in #586 if it solves your issue.
Please stop +1 in comments and use the 'Add your reaction' feature in the original description.
+1. Our client always cost 6~10s to wait for response from a provider. It is impossible to shorten the waiting time.
@adriancole - what are the versions of Java that OpenFeign supports? Could this not be completed by having the Client interface return CompletableFuture<Object>? Then based on the Contract and return type determined in the MethodHandler(It's called `SynchronousMethodHandler so probably needs a rename as part of this) unwraps the CompletableFuture or leaves it alone. Let me know if i'm going down the wrong track on that. I think that's the least invasive approach but is still breaking api compatibility and pins OpenFeign to jdk8+.
Alternative is to use callbacks i guess and detect them in the parameter list of the feign annotated interface and pass down to the Client. The CompletableFuture feels more clean.
Two things here:
A change to an interface is a breaking change. Feign supports java 6+ (android etc from whatever old version we used to support would presumably still work)
I'm not sure completable future is a great async api when we consider how backends compose with it. It sounds nice, and feels so, but for example, non-blocking has some rather dramatic impact. For example, it impacts other apis and how data is parsed etc. We'd likely end up with a frankenasync api which at the surface looks nice but isn't actually non-blocking, just deferring certain parts or spawning threads not to which may or may not be desired depending on what people mean. It might be more complex way of doing something similar to what's done in hystrix (which allows async on top of feign using threads), but more code.
I think the best way forward is actually to choose a real http async client implementation and pin to that. This simplifies how things at the functional composition can behave. This is why retrofit is easier to do async as well, as it doesn't have to consider the N-way concerns of async composition on top of N-way composition of async client backend.
I haven't reviewed this issue in a while but I recall someone did vert.x in the past. The fact that major surgery is involved might be a reasonable expectation.
At any rate I don't have new information, but knowing what I know I wouldn't expect Future to be a great choice for client implementation (even if it is decent for being a pack-in type). Something more like a callback or reactive streams mono or something would seem a more realistic client backend if swapability was to be maintained.
Pardon commentary prior to catching up on the issues in the past several months or however long I plan to review things at length between now and the weekend release https://github.com/OpenFeign/feign/issues/643