-
15. Re: Registering transaction synchronization during time of beforeCompletion
smarlow Oct 3, 2018 8:43 AM (in response to ochaloup)smarlow I'm not sure if it's safe to allow user to work with the interposed synchronization. From what I understand the WFLY needs to control the ordering of interposed synchronization callbacks (wildfly/JCAOrderedLastSynchronizationList.java at master · wildfly/wildfly · GitHub ) and allowing user to easily add interposed synchronization could break whole the thing. Or could not?
On CDI - I think the way how CDI can work with this is well summarized in [CDI-724] Transactional observers fired in inconsistent manner when synchronisation cannot be placed
Advanced applications can already lookup the TransactionSynchronizationRegistry to register an interposed sync, so this would be yet another way that is also mentioned already on https://issues.jboss.org/browse/CDI-724. If we agree that allowing users to express that a CDI transactional observer should be on interposed or non-interposed syncs, we should add a comment indicating so on CDI-724 (IMO).
Scott
-
16. Re: Registering transaction synchronization during time of beforeCompletion
mmusgrov Oct 3, 2018 9:07 AM (in response to tomjenkinson)These all sound like hacks to me. To get the behaviour you want the correct approach would be to introduce an extra interface method to register an afterCompletion callback:
public void registerSynchronization(AfterCompletionSynchronization sync)
and add a spec item that describes in which transaction states it is valid to call it. But I doubt you would get many votes for such a spec change.
-
17. Re: Registering transaction synchronization during time of beforeCompletion
ochaloup Oct 4, 2018 4:05 AM (in response to mmusgrov)smarlow ok, interesting. From implementation perspective I think it could be problematic in way that currently the JCAOrderedLastSynchronizationList should be last in the list as after that the JPA is not able to serve request for particular transaction. If user enlist new interposed synchronization from the JCAOrderedLastSynchronizationList#beforeCompletion then this synchronization will be called after the JCAOrderedLastSynchronizationList invocation finishes. Possibly it could be an issue for him.
mmusgrov I think I understand your point of view. Despite that I would like add that the specification does say nothing about enlisting new synchronization at time the interposed synchronization is called. If we consider it's not forbidden then some consequences comes for the caller.
tomjenkinson mmusgrov : currently I think about two possibilities
* take the number 3 and enhance Narayana SPI or go with the combination of the 2+3 as Tom suggested (plus in parallel asking about spec to be enhanced)
* leave the behaviour as it is and leave it for the user managing it by himself or him waiting for CDI specification change
-
18. Re: Registering transaction synchronization during time of beforeCompletion
tomjenkinson Oct 4, 2018 4:11 AM (in response to ochaloup)Number 3 is basically a non-spec equivalent of Mikes approach so I think we should try to go with that then we can make the recommendation to the spec group. Do you think we would be able to ask Weld to change to use a synchronization that extends from our SPI?
-
19. Re: Registering transaction synchronization during time of beforeCompletion
ochaloup Oct 4, 2018 4:36 AM (in response to tomjenkinson)tomjenkinson sure, I've already planned a small talk with manovotn from the WELD team. I will report back then.
-
20. Re: Registering transaction synchronization during time of beforeCompletion
mmusgrov Oct 4, 2018 5:29 AM (in response to ochaloup)mmusgrov I think I understand your point of view. Despite that I would like add that the specification does say nothing about enlisting new synchronization at time the interposed synchronization is called. If we consider it's not forbidden then some consequences comes for the caller.
tomjenkinson mmusgrov : currently I think about two possibilities
* take the number 3 and enhance Narayana SPI or go with the combination of the 2+3 as Tom suggested (plus in parallel asking about spec to be enhanced)
* leave the behaviour as it is and leave it for the user managing it by himself or him waiting for CDI specification change
The existing API for registerSynchronizaton/registerInterposedSynchronization includes the exception IllegalStateException which is thrown if the transaction is in the wrong state, the caller should be checking for this and react. I know that the javadoc currently says it is thrown if the transaction is in the prepared state/not associated but if people feel that this isn't good enough then the javadoc could be expanded to include the various edge states you want to cover. But I think this is overkill since your real goal is to be able to have the capability of registering for afterCompletion only.
Also I did not follow your explanation about why CDI cannot use an interposed synchronization.
-
21. Re: Registering transaction synchronization during time of beforeCompletion
ochaloup Oct 4, 2018 7:37 AM (in response to mmusgrov)mmusgrov I've studied the implementation of the `JCAOrderedLastSynchronizationList` in more details and I consider I haven't fully understood the functionality. The WFLY creates the implementation of the TransactionSynchronizationRegistry in way when interposed synchronization is registered then it's checked if it's class from `org.jboss.jca` and if so then such synchronization is called as the last in the chain of the synchronization callbacks. Otherwise it's added to the set of "normal" interposed synchronization callbacks and those are called in order they were registered for the beforeCompletion and in reverse order for the afterCompletion. So if CDI registered all the events observers as interposed ones it should not break the functionality or registering synchronization callbacks. But the issue is that the CDI event observer will be registered to be called after the JPA synchronization callback will be called. If the user defined event observer then calls some of the hibernate APIs it can be informed incorrectly about closed entity manager etc. It's just the user defined events should be called before the server intended interposed synchronization call. If user does not need to use the JPA objects then it would work fine otherwise there could be another issues (at least from my current point of view). As smarlow the CDI spec could be enhanced in way that user could create observer and define if the synchronization will be interposed or not. It could (partially) fix the issue as user has to deliberately declares it's interposed synchronization and then he bears consequences. It would solve this particular issue probably because afterCompletion the JPA state is possibly closed either. How challenging would be implement this proposal to CDI/Weld is then better to ask manovotn
When getting back. I've discussed the issue with manovotn (thanks a lot) and I found that "I think everybody's right, except me, so just forget I spoke, all. right?"
I've understood that the WELD is vendor independent and it can't depend on the narayana-spi in any sense. From that perspective we need to either take the option 1. - fill the request to the JTA spec forum (and go to option 4.) or take the option 2. make a configuration option in Narayana which optionally permits such behaviour or take the option 4. leave it as it is till the CDI specification is updated.
From the general consensus I feel the option 2. is hacky then I probably say to go with option 1. to write a message to spec to notify about the "discrepancy" and then leave it for CDI. What do you think?
-
22. Re: Registering transaction synchronization during time of beforeCompletion
tomjenkinson Oct 4, 2018 9:25 AM (in response to ochaloup)I think if we go to the spec group we should go with two things:
1. Get the wording clarified that you cannot register a session synchronization before an interposed synchronization - pretty much it says that but there seems room to be explicit
2. Propose the addtion of the public void registerSynchronization(AfterCompletionSynchronization sync) and that can be explicitly registered at any point prior to prepare and while transaction is active and define it is called after all session syncs during aftercompletion.
-
23. Re: Registering transaction synchronization during time of beforeCompletion
ochaloup Oct 4, 2018 10:13 AM (in response to tomjenkinson)tomjenkinson I see. Just to fully understand have you meant "you cannot register a session synchronization *after* an interposed synchronization", right?
-
24. Re: Registering transaction synchronization during time of beforeCompletion
tomjenkinson Oct 4, 2018 10:21 AM (in response to ochaloup)Yes - I did mean that thanks for the correction
-
25. Re: Registering transaction synchronization during time of beforeCompletion
mmusgrov Oct 4, 2018 1:57 PM (in response to ochaloup)ochaloup wrote:
tomjenkinson I see. Just to fully understand have you meant "you cannot register a session synchronization *after* an interposed synchronization", right?
That statement is too vague. I should be allowed to register an interposed sync and then register a session sync and then call commit on the transaction.
-
26. Re: Registering transaction synchronization during time of beforeCompletion
tomjenkinson Oct 4, 2018 3:03 PM (in response to mmusgrov)I was thinking that this new sync type would be not referred to as a session syncrhonization and so it shouldn't really affect it's introduction to the standard.
It is why I added it as a separate point that it seems under specified and could be tightened up but if it is causing confusion then let's just stick with the main point:
* Propose the addtion of the public void registerSynchronization(AfterCompletionSynchronization sync) and that can be called at any point prior to the transaction being prepare (and while it is still active). Define that AfterCompletionSynchronization are called during afterCompletion after all interposed and session syncs have executed.
-
27. Re: Registering transaction synchronization during time of beforeCompletion
ochaloup Oct 9, 2018 6:22 AM (in response to ochaloup)After some next discussion I agree that the proposal for the specification change would start to make specification difficult to understand and it's not elegant.
What I agree is that CDI could use interposed synchronization as other WFLY component does - namely JPA and JCA. The specification says on TransactionSynchronizationRegistry :
"This interface is intended for use by system level application server components such as persistence managers, resource adapters, as well as EJB and Web application components."
which implies the CDI as system component could (or should) use it.
I'm leaving next discussion to the spec change CDI-724 or at the WELD issue WELD-2444.
-
28. Re: Registering transaction synchronization during time of beforeCompletion
jwgmeligmeyling Nov 30, 2018 7:45 AM (in response to ochaloup)Joining in rather late here, but I hadn't noticed that my issue had been picked up recently. I have found yet another issue related to transaction synchronizations, namely the registration of the transactional context cleanup synch for the transtacted JMS context: [WFLY-11450] Cannot get delegate of JMSContext during an beforeCompletion synch - JBoss Issue Tracker . I still would like to see these two issues to be resolved. What exactly is the general consensus here? Wait for more feedback? Wait for a community fix? I'd certainly be willing to contribute, if pointed towards the right direction.
-
29. Re: Registering transaction synchronization during time of beforeCompletion
tomjenkinson Nov 30, 2018 10:31 AM (in response to jwgmeligmeyling)I think these are Weld issues so if you have not done so already it would be worth discussing it in their community: Weld
I found this which seems to be where they do the registration: Search · registerSynchronization · GitHub
Perhaps something in the transactionservices:
core/TransactionalObserverNotifier.java at master · weld/core · GitHub
Which is defined here:
api/TransactionServices.java at f88f340a748d65fa0b4e1283efc8de3295f7e6fb · weld/api · GitHub
(I haven't found the implementation of that class)
As Ondra mentioned above there is a CDI spec issue, You could try to coordinate on [CDI-724] Transactional observers fired in inconsistent manner when synchronisation cannot be placed - JBoss Issue Track… and it looks like they have a spec mailing list: CDI development mailing list | Contexts and Dependency Injection