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

Ensure that potentially blocking API's can be called asynchronously #24

Open
mmusgrov opened this issue Sep 14, 2018 · 10 comments
Open
Milestone

Comments

@mmusgrov
Copy link
Contributor

Ensure that potentially blocking API calls can be called in asynchronous mode. Some API methods such as LRAParticipant already do whereas others such as LRAClient do not,

@mmusgrov mmusgrov changed the title Ensure that all API's can be called asynchronously Ensure that potentially bocking API's can be called asynchronously Sep 14, 2018
@tomjenkinson
Copy link
Contributor

Can we do anything more on the API further than Futures. @mmusgrov mentioned composable futures perhaps for a sequence like startLRA, do something, closeLRA

Perhaps @xstefank has some input from his research?

@xstefank
Copy link
Member

The design is based on REST invocations which already provide asynchronous calls. Do we need anything more for the initial proposal? IMO the spec embedded execution may complicate / confuse first implementors.

@tomjenkinson
Copy link
Contributor

This is for the LRAClient right, as in the programatic API. Under the covers it could use asynchronous REST but this is at the Java API level? Or perhaps I misunderstood the query @xstefank ?

@xstefank
Copy link
Member

LRAClient is REST invocations (https://github.com/eclipse/microprofile-lra/blob/master/api/src/main/java/org/eclipse/microprofile/lra/client/LRAClient.java#L106). @mmusgrov already mentioned that programmatic API (LRAParticipant) already do support async invocations that's why I've described only REST. Sorry for the confusion @tomjenkinson .

@tomjenkinson
Copy link
Contributor

tomjenkinson commented Oct 26, 2018

I think what @mmusgrov is suggesting is to change things like:

URL startLRA(URL parentLRA, String clientID, Long timeout, TimeUnit unit)
throws GenericLRAException;

to:

Future<URL> startLRA(URL parentLRA, String clientID, Long timeout, TimeUnit unit)
throws GenericLRAException;

Then those rest invocations which can be async can be benefitted from in the business logic too?

@xstefank
Copy link
Member

I see, wouldn't it be better to provide both options? Like startLRA and startLRAAsync. Even if I am not sure it makes sense for everything. In this particular case for start of the LRA, if ServiceA starts LRA asynchronously, it can't make a call to ServiceB as it doesn't know the LRA id yet.

@tomjenkinson
Copy link
Contributor

That is a good point. startLRA puts the LRA on the thread (although implementations could use the future too and poll it before propagating?

I think if we do both we have to do both in LRAParticipant too right?

@mmusgrov
Copy link
Contributor Author

Two points, firstly, @xstefank, what precisely do you mean by "The design is based on REST invocations ..". The spec specifies annotations that application developers apply to their JAX-RS resource methods. Is that what you mean?

Secondly, yes I am referring to the LRAClient API. Perhaps we could delay the async version of the APIs to release 1.x which I guess would imply that LRAParticipant methods should be synchronous.

@xstefank
Copy link
Member

@mmusgrov 1/ yes, you are right. I also agree with 2/

@tomjenkinson
Copy link
Contributor

I also agree with 2 from a consistency point of view

@mmusgrov mmusgrov changed the title Ensure that potentially bocking API's can be called asynchronously Ensure that potentially blocking API's can be called asynchronously Oct 29, 2018
@mmusgrov mmusgrov added 1.x and removed 1.0 labels Dec 3, 2018
@mmusgrov mmusgrov added this to the 1.x milestone Dec 5, 2018
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

No branches or pull requests

3 participants