Skip to content
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

Run the method scheduled with the Quartz scheduler on a duplicated context #28348

Merged

Conversation

cescoffier
Copy link
Member

This is the follow-up of #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.

@machi1990 machi1990 requested a review from mkouba October 3, 2022 08:23
@machi1990
Copy link
Member

lgtm, thank @cescoffier for the followup.

let's get a review from @mkouba as well.

@quarkus-bot

This comment has been minimized.

@mkouba
Copy link
Contributor

mkouba commented Oct 3, 2022

I wonder if we should have a config switch (false by default) to use the Quartz thread pool for blocking scheduled methods instead?

@cescoffier cescoffier force-pushed the run-scheduled-methods-with-quartz-on-dc branch from db34336 to c69cba6 Compare October 6, 2022 06:50
@cescoffier
Copy link
Member Author

@mkouba implemented and updated.

@quarkus-bot

This comment has been minimized.

…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 cescoffier force-pushed the run-scheduled-methods-with-quartz-on-dc branch from c69cba6 to 9452ab3 Compare October 6, 2022 07:45
@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 6, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 6, 2022

Failing Jobs - Building 9452ab3

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@mkouba
Copy link
Contributor

mkouba commented Oct 6, 2022

@cescoffier Pls add a note to the Migration-Guide-2.14.

@mkouba mkouba merged commit cdca978 into quarkusio:main Oct 6, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Oct 6, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 6, 2022
@cescoffier cescoffier deleted the run-scheduled-methods-with-quartz-on-dc branch October 7, 2022 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants