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

Add @RegisterLRAParticipant annotation for explicit registration of participants #88

Open
xstefank opened this issue Feb 4, 2019 · 16 comments
Milestone

Comments

@xstefank
Copy link
Member

xstefank commented Feb 4, 2019

I would like to propose addition of new @RegisterLRAParticipant annotation which will denote a CDI bean which will be registered as LRA participant by the LRA implementation. This annotation will be a CDI @Stereotype with @Dependent scope (see REST client). The LRA implementation filters the classes with @RegisterLRAParticipant at boot time and scans their methods for LRA participant annotations (@Compensate, @Complete, @Status, @Forget, @Leave).

The LRAManagement or LRAClient method will then take as an argument in their joinLRA methods participant class (a class which is annotated by @RegisterLRAParticipant) or (if requested) we can add and optional String name parameter to the register annotation and add joinLRA variant that references participants by string names.

@mmusgrov
Copy link
Contributor

mmusgrov commented Feb 4, 2019

Would this require the presence of a deployment processor - if so I don't think we can rely on that.

@ochaloup
Copy link
Contributor

ochaloup commented Feb 5, 2019

@mmusgrov what do you mean by term deployment processor? The annotations need to be processed during the application startup. I think CDI extension machinery could be smoothly used for that.

@xstefank
Copy link
Member Author

xstefank commented Feb 5, 2019

@mmusgrov @ochaloup if it helps I can push my PoC where I tried these suggestions

@mmusgrov
Copy link
Contributor

mmusgrov commented Feb 6, 2019

@xstefank yes seeing the PoC will help me understand

@mmusgrov
Copy link
Contributor

mmusgrov commented Feb 7, 2019

Can we make this work without using LRAClient (see #95)?

@xstefank
Copy link
Member Author

xstefank commented Feb 7, 2019

@mmusgrov I believe so, it can be just declaring "global" JAX-RS participant so the implementation can process class once on startup.

@mmusgrov
Copy link
Contributor

mmusgrov commented Feb 7, 2019

Great

@rdebusscher
Copy link
Member

@xstefank

will then take as an argument in their joinLRA methods participant class (a class which is annotated by `@RegisterLRAParticipant`)

How does the CDI extension for example knows the LRA for the joinLRA method? Or is this only to register a global participant (one participant which handle all cancel/complete/... requests)

@xstefank
Copy link
Member Author

xstefank commented Feb 8, 2019

@rdebusscher Spec implementation CDI extension would just process and register all @RegisterLRAParticipant annotated classes on startup. Users would than be able to use registered participant classes as arguments to the LRAClient/LRAManagement#joinLRA methods.

I did not cover participants handling all cancel/complete requests in this proposal.

@mmusgrov
Copy link
Contributor

mmusgrov commented Feb 8, 2019

@xstefank The reason I asked

Can we make this work without using LRAClient (see #95)?

is because we would no longer have a programmatic way of joining an LRA so it would have to be a global handler.

@mmusgrov
Copy link
Contributor

mmusgrov commented Mar 3, 2019

@xstefank Can we close this PR. I think the consensus was to create an interface to hold all the relevant participant methods:

  • compensate
  • complete
  • leave
  • forget

The status method can be implemented either by a JAX-RS endpoint or by a proxy.

@xstefank
Copy link
Member Author

xstefank commented Mar 3, 2019

@mmusgrov This issue was proposed for CDI beans which I am not sure is possible to collect based just on the implementation of the interface. I would still like to see this functionality in the spec some day so would it be ok to just defer it to 1.x? Assuming that we will add clarification for the JAX-RS resources and implementation of the participant interface for issue #35

@rdebusscher
Copy link
Member

@xstefank @mmusgrov For me the @RegisterLRAParticipant was also the trigger to register the participant at startup/deployment of the application and not during first use. (when placed on a JAX-RS resource which has @LRA and @Complete and/or @Compensate or alike annotated method)

(this is not the same as a global participant where there is only 1 participant for all LRA endpoints.)

or should we create something like @RegisterAtStartup?

@xstefank
Copy link
Member Author

xstefank commented Mar 4, 2019

@rdebusscher I thought so also but @mmusgrov explained that for microservices we do not expect that many JAX-RS resources to be present so it should be sufficient to scan only for @Path annotated classes and check for LRA annotations. The @RegisterLRAParticipant would be now (as we are sticking with JAX-RS) only performance optimization which is possibly making spec less usable as users need to remember to place this annotation even when they are already required to place @LRA on at least one method.

On the other hand I would like to see pure CDI participants in later release (extracting participant logic to separate classes away from JAX-RS resource) and thus such annotation will be required.

I will add this item on todays meeting to rediscuss requirements for 1.0.

@rdebusscher
Copy link
Member

@xstefank

we do not expect that many JAX-RS resources to be present so it should be sufficient to scan only for @path annotated classes and check for LRA annotations.

I agree. But are these registered at startup/deployment time, or only after the first usage (as I always understood based on our discussions)?

Now that I think about the scenario more, it really doesn't matter. The coordinator will retry anyway and does not wait until the participant is registered again (for a different LRA). So participant needs to be available immediately after deployment of the app to receive @Compensateand/or @Completecallbacks.

@xstefank
Copy link
Member Author

xstefank commented Mar 4, 2019

@rdebusscher exactly, I will be adding TCK for this scenario if arquillian can somehow "kill" client service.

@rdebusscher rdebusscher modified the milestones: 1.0, 1.x Mar 4, 2019
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

4 participants