search

eclipse / microprofile-sandbox

Apache License 2.0
38 stars 54 forks source link

@NestedLRA instead of @LRA? #46

Closed nmcl closed 5 years ago

nmcl commented 5 years ago

Why have a different annotation for nested LRAs (@NestedLRA)? Surely from a modularity perspective it would be better to just use @LRA and if there's an LRA associated with the thread/unit-of-work when it encounters another @LRA then it is automatically nested? Then a developer doesn't have to worry about nesting and code can be moved seamlessly between applications and scopes without having to be modified.

Also what happens if an LRA isn't active when @NestedLRA is called?

rdebusscher commented 5 years ago

With the @NestedLRA you can implement a specific scenario which is not possible when using only @LRA(type=Type.NEW)

This is the current spec document (see https://github.com/eclipse/microprofile-lra/blob/master/spec/src/main/asciidoc/microprofile-lra-spec.adoc)

  • If the nested LRA is closed but the outer LRA is cancelled then the participants registered with the nested LRA will be told to compensate.
  • If the nested LRA is cancelled the outer LRA can be still closed.

So a NestedLRA is not completely independent from the 'parent' LRA in which it runs. If the parent is cancelled, the compensate actions for the NestedLRA are also called. But the NestedLRA can be cancelled although the 'parent' LRA can just be successfully closed.

When you try to implement it using only the @LRA, there is no possibility to the achieve the above scenario.

Also what happens if an LRA isn't active when @NestedLRA is called?

An exception should be thrown indicating this wrong usage status. But is isn't specified in the spec at the moment. I have opened https://github.com/eclipse/microprofile-lra/issues/117

mmusgrov commented 5 years ago

How about adding the NESTED option to the Type attributed of the @LRA annotation, ie

NESTED, REQUIRED, REQUIRES_NEW, MANDATORY, SUPPORTS, NOT_SUPPORTED, NEVER

kenfinnigan commented 5 years ago

Wondering if this issue should be moved to https://github.com/eclipse/microprofile-lra and the LRA code removed from the sandbox?

mmusgrov commented 5 years ago

+1 I'll do that as soon as @nmcl and the team have resolved this current question

mmusgrov commented 5 years ago

@nmcl we have created issue #117 for this in the microprofile-lra repo. Do you mind if we close this one in preparation for removing LRA from the sandbox.

mmusgrov commented 5 years ago

Moved to https://github.com/eclipse/microprofile-lra/issues/117