Skip to content

Make all Vert.x executeBlocking ordering requirement explicit#54314

Open
jponge wants to merge 3 commits into
quarkusio:mainfrom
jponge:fix/explicit-executeblocking
Open

Make all Vert.x executeBlocking ordering requirement explicit#54314
jponge wants to merge 3 commits into
quarkusio:mainfrom
jponge:fix/explicit-executeblocking

Conversation

@jponge
Copy link
Copy Markdown
Member

@jponge jponge commented May 19, 2026

This ensures each call to executeBlocking defines its ordering needs instead of inadvertently falling back to strict ordering.

@jponge
Copy link
Copy Markdown
Member Author

jponge commented May 19, 2026

Note: I've used my best judgement (most calls should use false so concurrency from the worker pool is used) but each extension expert should have a look. Also CI could reveal failing tests that I might have missed.

@Ladicek
Copy link
Copy Markdown
Member

Ladicek commented May 19, 2026

Is there a way to prevent using the API that makes the choice for you? I don't know how the ForbiddenAPI check works, but that might be an option? If we don't do that, it feels like we'll inevitably get it back in the future.

@jponge
Copy link
Copy Markdown
Member Author

jponge commented May 19, 2026

BytecodeTransformerBuildItem might be extreme, no? 🤣

@Ladicek
Copy link
Copy Markdown
Member

Ladicek commented May 19, 2026

ForbiddenAPI checks at compile time, which feels better than runtime to me :-)

@jponge
Copy link
Copy Markdown
Member Author

jponge commented May 19, 2026

I'm not sure which ForbiddenAPI you're talking about, do you have any pointer? I could stack a commit for that here.

@Ladicek
Copy link
Copy Markdown
Member

Ladicek commented May 19, 2026

We're already using this project https://github.com/policeman-tools/forbidden-apis in Quarkus, there's multiple references to it, grep for forbiddenapis. I think it should be possible to add the executeBlocking() signature(s) into .forbiddenapis/banned-signatures-common.txt and that should be it? (I'm saying "signatures", because we probably want to ban both Vertx.executeBlocking() and Context.executeBlocking().)

@geoand
Copy link
Copy Markdown
Contributor

geoand commented May 19, 2026

ForbiddenAPI checks at compile time, which feels better than runtime to me :-)

We've used ForbiddenAPI in various extensions haven't we?

@geoand
Copy link
Copy Markdown
Contributor

geoand commented May 19, 2026

We should definitely ban the version that doesn't require the boolean param

Copy link
Copy Markdown
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, but others should look at their respective parts as well

@jponge
Copy link
Copy Markdown
Member Author

jponge commented May 19, 2026

I've added a ban list and fixed a few more call sites.

It will have to be reworked for Vert.x 5 as the executeBlocking APIs have changed.

@quarkus-bot

This comment has been minimized.

Comment thread .forbiddenapis/banned-signatures-common.txt
Ladicek
Ladicek previously approved these changes May 20, 2026
Copy link
Copy Markdown
Member

@Ladicek Ladicek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand any of the modified code here, but the ban makes perfect sense to me 👍

@quarkus-bot

This comment has been minimized.

geoand
geoand previously approved these changes May 20, 2026
@quarkus-bot

This comment has been minimized.

@jponge jponge force-pushed the fix/explicit-executeblocking branch from 0c67b87 to f37ce4b Compare May 27, 2026 15:11
@jponge jponge force-pushed the fix/explicit-executeblocking branch from f37ce4b to 65fed83 Compare May 27, 2026 15:12
@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Copy Markdown
Contributor

geoand commented May 28, 2026

integration-tests/hibernate-orm-graphql-panache looks like a genuine failure

@jponge
Copy link
Copy Markdown
Member Author

jponge commented May 28, 2026

@geoand I've rebased and tried locally, and integration-tests/hibernate-orm-graphql-panache work fine "on my machine". I'm going to push and see what CI says.

@jponge jponge dismissed stale reviews from Ladicek and geoand via 50460d2 May 28, 2026 12:54
@jponge jponge force-pushed the fix/explicit-executeblocking branch from 65fed83 to 50460d2 Compare May 28, 2026 12:54
@quarkus-bot
Copy link
Copy Markdown

quarkus-bot Bot commented May 28, 2026

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 50460d2.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@github-actions
Copy link
Copy Markdown

🎊 PR Preview 8201058 has been successfully built and deployed to https://quarkus-pr-main-54314-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@quarkus-bot

This comment has been minimized.

jponge added 3 commits May 28, 2026 20:26
This ensures each call to executeBlocking defines its ordering needs
instead of inadvertently falling back to strict ordering.

Fixes quarkusio#54088
Also catch a few more executeBlocking call sites.
@jponge jponge force-pushed the fix/explicit-executeblocking branch from 50460d2 to b087527 Compare May 28, 2026 18:27
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.

Revisit Vert.x executeBlocking strict ordering by default

3 participants