Non-blocking I/O support

[rstml]

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.

[yairogen]

I would also like to see this supported.

[shimib]

+1

[igreenfield]

+1

[bahirsch]

+1

[codefromthecrypt]

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?

[igreenfield]

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.

[codefromthecrypt]

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!

[yairogen]

What do you mean by "which is what feign isn't designed for"?

[codefromthecrypt]

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)

[yairogen]

got it. don't think I can commit to so much work at the moment.

[daniellavoie]

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.

[codefromthecrypt]

probably want to collaborate on https://github.com/OpenFeign/feign-vertx for starters

On Sat, Sep 3, 2016 at 3:24 PM, Daniel Lavoie notifications@github.com 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 .

[daniellavoie]

Thanks @adriancole sounds interesting. I'll check it out.

[phymbert]

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.

[shenliuyang]

+1

[beihaifeiwu]

+1

[maheshmahadevan]

+1

[oldprofile]

+1

[SerJey]

+1

[iischwuerfl]

+1

[paskos]

+1

[spencergibb]

Please stop +1 in comments and use the 'Add your reaction' feature in the original description.

[dieppa]

+1

[stv8]

+1

[dieppa]

+1

[Ged0]

+1. Our client always cost 6~10s to wait for response from a provider. It is impossible to shorten the waiting time.

[bluntviola]

+1

[scarter93]

+1

[jmoney]

@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.

[codefromthecrypt]

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

[codefromthecrypt]

I just had an idea.. why don't we make "retrofeign" which allows feign to leverage retrofit 2's async layer? https://github.com/OpenFeign/feign/issues/644

[jmoney]

Agreed. It'd be great to have those options for asyncing things. But this ticket sounded initially about adding non-blocking support. Async in retrofit is still a blocking call, unlike using netty where it's completely non-blocking and async. But if we can make Feign support an async api, then it's just the client impl responsibility to map the non-blocking to the async api so i definitely think it's worth exploring. That was the attempt at suggesting a Promise style pattern(like CompletableFuture) but I'm willing to look at other examples. The crux is what is the Client interface look like for async. That kind of dictates the rest of the changes that need to happen.

[codefromthecrypt]

If this is about pure non blocking ex nio etc I think it implies netty and significant internal work. I dont see this happening since we arent even at a point of health maintaining the current codebase. In the mean time i would look at the vert.x stuff.

We really have to shore up the ship. There are a lot of +1s but honestly it is irrelevant to discuss huge things until we have multiple people doing small releases. Walk before run in other words.

[DanielYWoo]

This is really useful to send requests from an API gateway to its upstream backend servers. But, Feign is not designed for non-blocking, this is about big API change, actually a whole new set of APIs.

The simplest code below would be totally different if we return a Future, or put a callback as the second parameter, and the exception declared will have to be handled in the callback.

    @RequestLine("POST /tickets")
    CreateTicketResponse createTicket(CreateTicketRequest request) throws SomeException;

If someone is willing to design a whole new set of non-blocking APIs, that would be great, but I don't think this can be fully discussed in this ticket.

[tshah010-zz]

Hi, I am using spring-cloud-starter-openfeign 2.0.1 and I am trying to make several http API calls asynchronously using org.springframework.scheduling.annotation.EnableAsync and org.springframework.scheduling.annotation.Async on methods annotated with @FeignClient. But none of the methods run async. Is it because @FeignClient is not allowing new threads to be spun up? Alternatively, I tried using on CompletableFuture on my FeignClient method but method never returns completed. What's the best way to make async calls when I am using spring-cloud-starter-openfeign? Thanks for your help.

[kdavisk6]

@tshah010

Feign does not support async execution modes currently. You can't do this yet. It's on our road map.

[tshah010-zz]

@kdavisk6 thanks for quick response, but Feign documentation for Hystrix here - https://github.com/OpenFeign/feign/tree/master/hystrix clearly says we can use CompletableFuture. But my methods never complete with and without @Async. Is that what you mean by "does not support"? Thanks Kevin.

[kdavisk6]

@tshah010

I mean that Feign has not been tested or built to support use within any async or non-blocking IO execution mode. The only fully-supported mode is blocking. The one note-able exception is Hystrix, however support is limited to use cases supported by Hystrix, such as using fallbacks and the HystrixFeign builders as the request management is delegated to Hystrix.

I'm afraid that's all I can offer you right now.

[motinis]

Please take a look at the the following pull request

[wangzihaogithub]

Does the current version work with CompleteableFuture? I can't find a use case.

[motinis]

Yes, it does. The pull request was merged after review and modifications. This enables the use of async clients with Feign.

On Mon, 28 Sep 2020 at 19:11 王子豪 notifications@github.com wrote:

Does the current version work with CompleteableFuture?

I can't find a use case.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OpenFeign/feign/issues/361#issuecomment-700133567, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEAZAM2QANCWRHY7EH5QTK3SICYTRANCNFSM4B6H2G3Q .

-- ‏נשלח מ-Gmail לנייד

[michalko]

+1

[velo]

Anyone willing to write a first draft for this?

[bob-bbb]

+1

[icarodemorais]

+1

[velo]

Yeah, still waiting on volunteers to either spear head this initiative or to fund it.

[wplong11]

@velo I worked on coroutine support(#1706) with minimal effort, and now I want to support full asynchronous features, and increase the stability level of AsyncFeign from Experimental to Stable. I'm not knowledgeable perfectly about Feign, but I've seen almost all of the internal implementations of Feign, FeignX, and ReactiveFegin, and I think I can try a few.

I plan to

1. Refactor the internal implementation to support full asynchronous functionality while reusing most of the code.
2. Add full asynchronous functionality
3. Clean up AsyncFeign Api Design

[velo]

@wplong11 feel free to reach me any time you find a blocker

[wplong11]

@velo Do you have any thoughts on what additional work must be done to increase the stability level of AsyncFeign from Experimental to Stable?

What I'm thinking of is:

• [ ] Modify AsyncFeign.AsyncBuilder to extend Feign.Builder or Modify Feign to support the features of AsyncFeign
  • It is intended to be used when customizing Feign.Builder in Spring Cloud OpenFeign.
• [ ] Modify CoroutineFeign.CoroutineBuilder to extend Feign.Builder

[wplong11]

@velo @motinis Is the parameter Optional<C> requestContext important? I understand the intention(#1174), but are you actually using it? It is not being used by AsyncClient implementation.

https://github.com/OpenFeign/feign/blob/f71849d38746fdc7871bf3b0609c488b36459687/core/src/main/java/feign/AsyncClient.java#L41

I have some problem to integrating AsyncFeign and Feign. It would be nice if Feign could support non-blocking asynchronous methods without AsyncFeign as well, but it's not easy to make appropriate changes while keeping the parameters. There's room for a little more thought, and I have something to explain more, but before that, I'd like to ask a quick question.