-
Notifications
You must be signed in to change notification settings - Fork 77
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
CDI context propagation #474
Comments
Note that other EE specs that have propagation within MP CP also don't have it mentioned in the specification - for instance JTA. The whole effort was MP oriented and by adding it to the CDI specification, it will outreach into SE or EE environment which I am not so sure is a good thing.
Yes, most of the issues are captured in this discussion and they still hold true so the discussion is definitely worth a read. |
Does this https://issues.redhat.com/browse/CDI-452 fall under this issue or is it different? Asking as to not open a duplicate. |
@cen1 I think the issue https://issues.redhat.com/browse/CDI-452 falls under this issue. |
I think JTA is more an example of why it needs to be defined in the Jakarta EE spec. Because context propagation isn't considered in the JTA programming model there are severe shortcoming with respect to usability and behavior that you do or don't get across different transaction manager implementations for propagated transaction context. For example, it has constantly been a problem for us that the JTA transaction continues to be associated with the original thread, possibly overlapping the completion stage action when it does actually run on another thread. A vendor-specific mechanism is needed for it to even be possible for the application to do a suspend of transaction context, but even that doesn't fully clear up this problem except when the application goes to great lengths to work around it. And because each transaction manager supports transaction context propagation to a different degree, everywhere that applications rely on it is non-portable code. Standardization is needed on the Jakarta EE side, and lacking that MicroProfile just did the best that it could.
I'd argue that it's definitely good to have the CDI spec standardize what and how CDI context behaves when accessed asynchronously and when/whether it is possible for which types of CDI context to be made available this way. Having this stanardized means that users would be able to write portable applications that can rely on getting consistent and predictable behavior, per the guarantees of the CDI (plus MP Context Propagation or Jakarta Concurrency) specification regardless of which providers are used. It would mean that the same CDI experts who are writing the CDI spec would have participation and ownership in defining how CDI context is or isn't to be used asynchronously and that when designing new capabilities in CDI, the CDI spec would continue to account for this rather than introducing new capability without any consideration for it and leaving MP Context Propagation/Jakarta Concurrency to later discover and deal with the mess after the fact, which might or might not even be possible. MicroProfile didn't engage the Jakarta CDI community originally for the same reason it didn't engage with Jakarta Transactions, which is that Jakarta EE didn't exist yet and Java EE was in limbo at the time. But now that Jakarta EE is here, all of these specifications will be made better if Jakarta EE can officially standardize requirements, behavior and expectations. |
For batch, something like Even some of the enhancement ideas adding scopes to share more within the job execution wouldn't do this. One subtle question: could batch's use case be satisfied by the spec tying the capability to a concurrency API like ManagedExecutorService (assuming CDI would even want to do that) ? I think probably it could, though I guess another approach from the CDI perspective might be to define context propagation more generally (w/o requiring ManagedExecutorService). Just mentioning. |
I promised to share a slide deck that I have that illustrates known troubles with request context propagation. Here goes: https://docs.google.com/presentation/d/1EaMdHY9NkIGn1qx3KYyk9hjCnAfLGoer7n3HN-GgoTc/edit |
We discussed this on the CDI weekly call two weeks ago and came up with quite a few issues. I'll write up what I remember, if anyone else has any corrections or more detail then please add that. The problem isn't well definedCDI doesn't have one context which may be propagated, it has several scopes and each have different rules. Any proposal must be more specific than "please propagate context". Issues with propagating the request context between threadsOne interpretation of this request for "CDI context propagation" is that the request context should be propagated to any task submitted from the request thread to allow code such as the following to work: @Resource private ManagedExecutor executor;
@Inject private MyRequestScopedBean bean;
public void myMethod() {
bean.setValue("foo");
executor.submit(() -> {
System.out.println(bean.getValue()); // prints foo
});
} We noted a number of problems with this.
Point 1 is the most pertinent. For this reason, we didn't think that propagating the request context between threads would ever be a good idea. Different threading models place different requirements on things which are propagated between threadsIn a reactive environment, operations will likely be broken up into lots of small peices of code which run asynchronously, but are guaranteed not to run concurrently. No blocking operations are allowed so it's very common for even a simple task to be broken into several asynchronous operations. In a thread pool environment, blocking operations are allowed and operations usually run on a single thread unless several things need to be done concurrently. When submitting an asynchronous task, it's expected that it may run concurrently with the current thread. If we define that some context is must be propagated in a particular way, we must make sure it makes sense for these different models. Conversely, it may make sense for some context to be propagated in one model but not in another due to the different ways that applications are written. Custom scopesSome of the requested features could be implemented using a custom scope, which can be done today using existing CDI APIs. For example, with a hypothetical |
Very nice summary, thanks! Ad item 3: the request context in Weld is not always just a thread-local |
Ladislav is right (there are actually 4 implementations of req. context based on where it is used). |
I think defining the problem is key here. One of them which I encountered often was propagating headers from incoming request to async rest clients. In non async world, I would usually inject the request context into the client implementation bean and add the headers there. If I wanted to use the client implementation in async it would fall on it's head and I would need to rewrite the implementation to pass all the headers through method params instead. This is obviously very ugly and frustrating when you need to do it. While this case could be perhaps solved with context propagation through CDI there is another approach. For example, mp-rest-client allows you to propagate headers from incoming request but they simply give you the header map and not the whole context to solve this specific problem (e.g. org.eclipse.microprofile.rest.client.ext.ClientHeadersFactory). Essentially what you would need to do manually in JAX-RS client but packaged in nicer way. So perhaps these problems that appear to require propagation can mostly be solved in different specifications with very narrow solutions instead. |
The Context Propagation lib was used in the latest Spring 6.0/Spring Boot 3.0. https://micrometer.io/docs/contextPropagation |
Curious where this stands at the moment. Is this being looked at for a subsequent release? //cc @Emily-Jiang |
I am not sure I see how is this related? It looks like micrometer custom solution to propagating thread local values?
This comment sums it up well - #474 (comment) |
@manovotn Currently CDI context propagation discussion focus on the context inheritance between scopes. But in a Jakarta EE application, we could encountered issues when switch to execute on different thread executors, eg. an async thread, Kotlin Coroutines, how to make beans available when switched to different execution context(async context, reactive context, or coroutines context). For example, in a In the new Spring 6.x, it addressed this issue via https://micrometer.io/docs/contextPropagation (which is mainly maintained by Spring guys) |
Maybe the way out is to rely on structured concurrency in the future? I know that that's a long way off until CDI can be based on some Java version that isn't even out yet, but the principle of structured concurrency solves some of the problems presented here, doesn't it? What I mean by that: The CDI container could provide a (probably Having the StructureTaskScope as an enclosing bracket also enables to capture the HttpRequest / backing map where the TaskScope is started and to allow read/write access to it from subtasks started within the scope. Of course, this does not solve the problem that existing |
Yes, that would solve one of the problems, partially (only for cases where structured concurrency is a good fit, which should be like 90%, but definitely isn't 100%). |
More broadly, the request scope (and the concept of normal scopes, in general) was designed for the thread per request paradigm. It was in the days when Java EE prohibited (or discouraged, stricly speaking, because there has been no way to actually prevent it) users from creating their own threads, and there were no managed threads / executors. The Concurrency Utilities for Java EE specification (Jakarta Concurrency, nowadays) has since the very beginning recommended to not use This is increasingly not good enough for today's needs, but based on our experience, I'm fairly convinced that MicroProfile Context Propagation (which was adopted into Jakarta Concurrency verbatim) is not a good solution. What is a good solution, I don't know (yet :-) ). |
In CDI spec, the context propagation was not addressed in the spec. The original ticket was raised:
In MicroProfile Context Propagation, some effort was made to address the context propagation and the implementation was made in Weld. However, it has a some issues, such as eclipse/microprofile-context-propagation#167
The text was updated successfully, but these errors were encountered: