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

AFTER_SUCCESS Observers and Events fired from within EntityListeners #467

Open
codylerum opened this issue Feb 17, 2021 · 12 comments
Open

Comments

@codylerum
Copy link

There currently exists an issue where if a event is fired from within an JPA EntityListener (@PostUpdate for example) the event may not be reliably observed via an @Observes(during = TransactionPhase.AFTER_SUCCESS)

The issue comes down to what state Transaction is in when the entityManager.flush is called (trigging the post update for example). If the entityManager.flush is called manually then the @PostUpdate is run while the transaction is active and things will work fine. If however, the flush is automatically called via the ending of an @Transactional method then when the @PostUpdate is run the transaction has moved past the ACTIVE status and the synchronization cannot be added to the transaction. No error is thrown and the event will silently not be observed.

Possible thoughts were having the the CDI event registered with an TransactionSynchronizationRegistry#registerInterposedSynchronization

It appears that you could do something like

  @PostUpdate
    public void postUpdate(Ticket entity) {
        try {
            TransactionSynchronizationRegistry registery = (TransactionSynchronizationRegistry) new InitialContext().lookup("java:comp/TransactionSynchronizationRegistry");
            registery.registerInterposedSynchronization(new MySyncronization());
        }
        catch (NamingException e) {
            throw new RuntimeException(e);
        }
    }
public class MySyncronization implements javax.transaction.Synchronization {

    static final Logger log = Logger.getLogger(MySyncronization.class);

    @Override
    public void beforeCompletion() {

    }

    @Override
    public void afterCompletion(int status) {
        if (status==3){
             CDI.current().getBeanManager().fireEvent(EntityEvent.updated(entity));
        }
    }
}

This appears to accomplish what we want, but obviously is not friendly to use.

The current thought is to potentially clarify in the spec that CDI events should use an Interposed Synchronization either all the time or only if a standard synchronization cannot be obtained, but that may have ramifications that I am not considering.

This jumps off from https://issues.redhat.com/browse/WELD-2444

@codylerum
Copy link
Author

Another thought here could be to have @Transactional call it's entifyManager.flush() while the transaction status is still ACTIVE

@manovotn
Copy link
Contributor

@codylerum thanks for creating the issue and restarting the discussion.

I did re-read the issue and all other links but I am not JTA expert (so apply a pinch of salt, please) - from this discussion that @ochaloup started after WELD-2444 I got the impression that using interposed synchronization would be against some of the bits in JTA spec.
Maybe @ochaloup could confirm or deny that ;-)

That being said, the way Weld implements Synchronization registration now is that integrators (such as WildFly) need to provide an impl of TransactionalServices which contains a registerSynchronization method. Technically speaking, I think they could just register interposed transaction instead of standard one, but apparently, no integrator did that.

@jwgmeligmeyling
Copy link

Its just not very clear from the Transactional Observer specification that it relies on Transaction Synchronizations and thus have the same limitations (can only be placed while the transaction is in state active). Due to the abstraction over Transaction Synchronizations, its not at all trivial that events with transactional observers can only be fired from an active transaction. One way or another, a user could eventually end up with a transactional observer trying to fire an event for another transactional observer which currently does not work. While JTA throws exceptions if the Synchronization cannot be placed, CDI implementors seem to swallow exceptions while firing events, thus the issue could go unnoticed for a while.

I don't think implementors necessairily need to use interposed synchronisations. Its also possible to process all transaction observers from a transaction scoped queue within a single transaction synchronization that is always registered with an active transaction.

The CDI spec just needs to mandate what needs to happen. Either implementors need to implement a way of supporting events with transactional events to be fired in transactions out of state active, or a clear exception needs to be thrown that firing an event in such scenario is not supported.

@manovotn
Copy link
Contributor

Its also possible to process all transaction observers from a transaction scoped queue within a single transaction synchronization that is always registered with an active transaction.

I am not sure this is a legitimate approach from the perspective of JTA. It sounds like it would violate the isolation of each OM invocation in context of transaction. But again, I am not sure and would love to hear a JTA expert opinion on that.

@jwgmeligmeyling
Copy link

I'm not sure what you mean with OM invocation, but I was talking about the way transactional interceptors can be implemented in CDI, not how synchronizations should be changed in JTA. Anyways, lets see what the JTA folks have to say 😄

@scottmarlow
Copy link
Contributor

What is the question for the JTA folks exactly? CC @tomjenkinson

Is there a question here also for the JPA team as well? CC @lukasj

@jwgmeligmeyling
Copy link

jwgmeligmeyling commented Feb 22, 2021

@scottmarlow

If the CDI specifications mandates that for every transactional observer a separate JTA Synchronization ought to be used, it would be useful to have a decicated API for afterCompletion synchronizations that can be placed during other beforeCompletion synchronizations (so outside transaction state ACTIVE).

So that during another beforeCompletion synchronisation (in the example, the JPA flush synchronisation), events with transactional observers in the AFTER_COMPLETION phase can still be registered.

@tomjenkinson
Copy link

I think a nice Jakarta Transactions RFE (@mmusgrov comment https://developer.jboss.org/message/985544#985544) would be to provide a Synchronization type that only allows afterCompletion and allow that that Synchronization type be registered prior to the start of the two-phase transaction commit process.

@manovotn
Copy link
Contributor

@tomjenkinson not sure I follow - how would that affect CDI spec, could you expand on it?
Does that mean we would always register that synchronization or just use it as a fallback in case the current synch. registration fails?

@tomjenkinson
Copy link

If you don't have require any implementation to be performed in beforeCompletion then it could always use that new type of Synchronization. Do you have any implementation you need to execute during beforeCompletion?

@manovotn
Copy link
Contributor

manovotn commented Feb 24, 2021

@tomjenkinson Yea, so currently you can bind an observer to phases listed as enum here. Most important parts are BEFORE_COMPLETION and AFTER_COMPLETION, the rest is easily done with the former two. In JTA terms this should translate into to a synchronization that's using either beforeCompletion or afterCompletion method. Such synchronization object can be seen here (link to Weld code).

@tomjenkinson
Copy link

So it seems that at least some implementations in Weld which needs beforeCompletion.

I don't think it will be easy to make Jakarta Transactions such that it allows adding in more Synchronizations during beforeCompletion but you could ask on the Jakarta Transactions mailing list for input. I am thinking that this feature:

  1. Could be complex to understand, in that if a user registers the Synchronization during beforeCompletion either we would possibly decide some concept of ordering would now take place and the Synchronization would be called after previously registered ones or perhaps we would decide the beforeCompletion will silently not be called or something else
  2. Might be restricted somehow by distributed use cases where the coordinator implementation is remote

If there are a discrete set of observers identified that only have the need for informing after the transaction completes then a smaller Jakarta Transactions change to allow a new synchronization type that be registered that is only called after a transaction completes and that can be registered while before completion is executing could be helpful and possibly added to that specification.

I think discussing the use case on the Jakarta Transactions mailing list would be useful.

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

5 participants