Executes scheduled methods on duplicated contexts #28255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove the executor from the ScheduledContext as it was confusing and can only be used for blocking methods - Create a duplicated context for both blocking and non-blocking invocation - Blocking invocation use executeBlocking, which uses the same thread pool as the previous executor - It is not possible to create the duplicated context in the thread factory (an idea discussed with Bruno and Martin because it creates a chicken-and-egg problem) - Simplify the dev console @scheduled support to use the same strategy - no need to play with the classloader anymore as there is no more in-thread direct invocation and the Vert.x thread that will be used has the right TCCL configured
|
Should this be backported? |
This comment has been minimized.
This comment has been minimized.
|
@gsmet no, I would consider that as a new feature. |
Did you double-check?
Note that this would be a breaking change because of a different thread used to execute a scheduled method. We should also ask @machi1990 for opinion - there might be some quartz features that rely on execution in the built-in thread pool. |
mkouba
approved these changes
Sep 29, 2022
|
@mkouba yes, I double check on which thread pool things were and are executed: it's the same. |
machi1990
approved these changes
Sep 29, 2022
mkouba
added
the
triage/waiting-for-ci
@holly-cummins JDK 17 MacOS M1failure ^ |
quarkus-bot
bot
added
kind/bugfix
and removed
triage/waiting-for-ci
cescoffier
added a commit
to cescoffier/quarkus
that referenced
this pull request
Oct 3, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
cescoffier
added a commit
to cescoffier/quarkus
that referenced
this pull request
Oct 6, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
cescoffier
added a commit
to cescoffier/quarkus
that referenced
this pull request
Oct 6, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
igorregis
pushed a commit
to igorregis/quarkus
that referenced
this pull request
Oct 16, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
igorregis
pushed a commit
to igorregis/quarkus
that referenced
this pull request
Oct 16, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
igorregis
pushed a commit
to igorregis/quarkus
that referenced
this pull request
Oct 16, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
igorregis
pushed a commit
to igorregis/quarkus
that referenced
this pull request
Oct 17, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
tmihalac
pushed a commit
to tmihalac/quarkus
that referenced
this pull request
Oct 27, 2022
…ntext. This is the follow-up of quarkusio#28255. When running the scheduled method, the lack of duplicated context can lead to contextual data loss (loss only, no leak). This commit makes sure that the method runs with a duplicated context. However, this is a breaking change for users expecting the method to run on a Quartz thread.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #28017
A follow-up for Quartz will be implemented later.