Skip to content

Comments

fix: disable batching for vt executor#32785

Merged
patriknw merged 4 commits intomainfrom
wip-disable-batching-for-vt-executor
Aug 15, 2025
Merged

fix: disable batching for vt executor#32785
patriknw merged 4 commits intomainfrom
wip-disable-batching-for-vt-executor

Conversation

@johanandren
Copy link
Contributor

@johanandren johanandren commented Aug 14, 2025

This fixes that batching in the dispatcher would cause new tasks scheduled from a virtual thread to be batched and run in the same virtual thread rather than immediately start in a separate virtual thread.

val all = threadInfoProbe.receiveN(4)
val items = all.collect { case (evt: String, threadId: Long) => (evt, threadId) }
items.map(_._2).toSet should have size (2) // two different virtual threads
items.map(_._1) shouldEqual (Seq("parent before", "child before", "parent after", "child after"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it can be flaky. It might take more than 20 millis for the child to be scheduled and then "parent after" will come before "child before"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, hmm, I couldn't figure out a way to make it completely deterministic without also making the reproducer go a way (like cross future synchronization something). Should I just tune the timeouts up a bit, I had them at 100 - 400 to start with but tuned down.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, some longer would be fine. Probably a valid case for dilated as well, so it takes some longer in CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More generous now

@johanandren
Copy link
Contributor Author

Note, no PR validation on JDK 21, so tests are disabled. I ran them locally and all green now (plus failing if I remove that marker from the VirtualThreadConfigurator)

Copy link
Contributor

@octonato octonato left a comment

Choose a reason for hiding this comment

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

LGTM

@johanandren johanandren requested a review from patriknw August 15, 2025 06:50
Copy link
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM

@patriknw patriknw merged commit 228843a into main Aug 15, 2025
5 checks passed
@patriknw patriknw deleted the wip-disable-batching-for-vt-executor branch August 15, 2025 06:53
@patriknw patriknw added this to the 2.10.9 milestone Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants