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
Context propagation: use new APIs from latest snapshots #3256
Conversation
simonbasle
commented
Oct 27, 2022
- Depend on post-RC1 snapshots of context-propagation
- Use ContextSnapshot.setAllThreadLocalsFrom
- Remove registry and intermediary classes, use Supplier
With the introduction of removal methods in ContextRegistry, it becomes more practical to use the ContextRegistry.getInstance global instance. Following that, two of the three intermediate classes are no longer needed and can be replaced with lambdas: ContextCaptureFunction and ContextRestoreHandleConsumer. Instead of calling the ContextPropagation methods with a subscriber (which necessitates some mocking in tests), we now take a Supplier of Context. A CoreSubscriber::currentContext method reference can do the trick in production code.
} | ||
|
||
static <T, R> BiConsumer<T, SynchronousSink<R>> contextRestoreForHandle(BiConsumer<T, SynchronousSink<R>> handler, CoreSubscriber<? super R> actual) { | ||
if (!ContextPropagation.isContextPropagationAvailable() || actual.currentContext().isEmpty()) { | ||
static <T, R> BiConsumer<T, SynchronousSink<R>> contextRestoreForHandle(BiConsumer<T, SynchronousSink<R>> handler, Supplier<Context> contextSupplier) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a small comment that the supplier can be overhead for the mono case since one extra object is allocated per subscriber * per operator. I guess that the goal was to avoid context retrieval if there is no need, but not sure what brings more impact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine since the Supplier
in the hot path is CoreSubscriber::currentContext
(ie. a method reference).
Late LGTM 👍 |
this also fixes #3244 |