thanks for your response. I added my feedback inline.
I fear, I got your last point (What about changing default behavior for the local jar) wrong and clarified my statement (see below).
Le 25 mars 2015 à 20:28, Mark Paluch <mpaluch@paluch.biz> a écrit :
Hi Antoine,
thanks for your input and this summary.
I placed my comments inline.
Best regards, Mark
Am 25.03.2015 um 14:45 schrieb Antoine Sabot-Durand <antoine@sabot-durand.net>:
Hi all,
This mail is quite long, but if you want to catch up on this double end activation for async event and bring your help on this point, you should take the 10 mn to read it and make your feedback. We’ve been talking of this for more thant on month now, so it’s normal that reflection and proposition take a few lines to synthesize
Discussion is going back to solution avoiding this double activation stuff for async event. To avoid explaining again why we should care and the solution we already explore here is a small wrap up of previous episodes :
1) Why is it important to take time on this?
Some of you may find we already spend too many time on this question, but remember. Async events are the 1st requested stuff from the community. It has been asked for a long time (Jira ticket is CDI-4). We didn’t provided a solution for CDI 1.1 so now people are waiting this feature and they probably hope it’ll be nicely designed.
To make short : if we don’t deliver users will be very disappointed, if we propose a lousy solution people will be very critic. I know that it’s better to not deliver than delivering something we are not happy with, but we really should be careful here
+1 CDI 2.0 without async events seems a no-go
2) Why this double activation is needed?
For the producer (fire()) side it’s rather obvious : we cannot magically change all synchronous event call to async. We need an handle on the work in progress (so a new method signature with CompletionStage), the payload mutation mechanism would break as all transactional events. So there’s no debate on fireAsync()
+1 on fireAsync
fireAsync triggers always asynchronous event processing. Currently we’re talking about interference between observers, but there are lots of other ways to do nasty things (e.g. modifying the payload from the firing thread). Any caller expects a method returning CompletionStage returning instantly, before any work is done. This leads to the point that disabling async on legacy observers would lead that fireAsync would return after those legacy observers are processed.
yes, it will behave like a standard fire().BTW the interesting part of my paragraph is "why it is needed on observer". As you seem to exclude that constraint in your following proposal, I’d like to read why you think so
I get your point, we want to restore old behavior in certain cases although observers are called using fireAsync and in the end it’s still sync (somehow). The invoker of fireAsync needs to be aware of what he/she is causing by that and that observers of a certain event might not be capable (yet) of handling the async nature. Same would apply if any client code would be called by an external authority in a callable/runnable instead of a sync way.
A happy coder’s approach of "hey, there’s a fireAsync, let’s use it" on existing events does not work here, too much can be broken. If someone uses a certain functionality, he/she must be aware of the consequences. I like the approach of providing the new fireAsync method, so you can explicitly opt-in.
3) Implementing this observer activation
I’m listing here all the solution to deal with this requirement. For some of them I’ll add the reason we won’t adopt it or my feeling about it
I would add another case which might help to clarify @Priority as well:
1. fire with @Observers: same behavior as CDI 1.x
2. fireAsync with @Observes: observers are processed in any order in sequence but in a different thread
How do you deal with transactional observer or observer injecting contextual beans?
I’d propagate the context/scope of contextual beans from the caller of fireAsync. These instances behave like they would have been fired the old way. I’m aware that especially @RequestScoped and even @SessionScoped (plus custom scopes) can get cleared while an async event is being processed. It’s a no-go raising context not active during the request. Creating new instances might be not useful since these instances can carry data which is needed for processing. Same applies for transactional observers. I understood from Jozef, that some transactional observers are notified already on different threads which is sort of semi-async. It’s at least decoupled from the firing thread.
- nothing new until now -
3. fireAsync with @Observers and @Concurrent (obviously not async to prevent confusion): Observers carrying @Concurrent are running concurrently to each other (that’s the double-opt in case). Observers not having @Concurrent are handled like point 2. observer methods are not allowed to to have both, @Priority and @Concurrent
4. fire with @Observes and @Concurrent: Ability to use fire with some level of async. The behavior on the caller side does not change, fire waits until all observer method invocations are finished
We should think also about ProcessObserverMethod. That’s the place where you can override any priority/(async|concurrent) settings and perhaps even the asyncSupported flag. This sort of opt-out is maybe sufficient. Now what happens, if asyncSupported=false and fireAsync? How about just skipping the observer?
How can you choose observer to opt out? Do you suggest a case by case approach? Won’t it be a bit complicated for standard end user to add an extension for that.
I expect the opt-out case as exception. Therefore this approach. I’m with you, if the opt-out becomes a standard case, an extension is too complex.
Mark, you’re describing additional interesting features for async event, but don’t react on the necessity of async support flag on observer and what would be your favorite choice.
I’ve a different viewpoint on the necessity of the async support flag on the observer. If you decide to use fireAsync on the one side (which might be located in framework code not under own control) and you decide to use a newer version of the framework jar, you’re required to adjust your code to keep it working. Blindly using fireAsync is a no-go from my perspective.
I like @ObservesAsync since it is a marker for new functionality. We do not pollute the class path with a second @Async and since we have already double opt-in (fire and @Observes) it would fit into the current patterns.
4) What about changing default behavior for the local jar?
Idea launched by some of us. We could extend the chosen scenario by activating AsyncSupport by default on all observer on the current jar (BDA). Since the main backward compatibility issue is linked to have different CDI jar from different owners and version we could give local control to the user for his own code and jars.
That could be done in beans.xml like we did for bean-discovery with an async-event attributes for instance or in code by a config annotation or event in extension (but we probably should expose the BDA concept in SPI if we go that way…)
This feels like a dirty hack and in the end you can’t tell how your application will behave.
It’s like your ProcessObserverMethod solution but limited to one Bean Archive that user controls. It’s only a convenient way to replace async opt-in addition on 99 observer by one opt-out on 1 observer…
I oversaw the bean archive limitation. If we really expose more parts of BDA in the SPI it can lead to some generic control over CDI for the deployment of the user and has the chance of getting a sort of control center for CDI within one deployment (control over multiple BDA’s). Could be also useful for specific enabling/disabling of extensions in the future.
5)Conclusion:
Now you have the whole picture. If I missed things, tell me. If you like an idea please tell it, if you have a new idea or a different POV, feel free to speak.
Thanks for reading.
Antoine
_______________________________________________
cdi-dev mailing list
cdi-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/cdi-dev
Note that for all code provided on this list, the provider licenses the code under the Apache License, Version 2 (http://www.apache.org/licenses/LICENSE-2.0.html). For all other ideas provided on this list, the provider waives all patent and other intellectual property rights inherent in such information.