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

add FaultTolerance.BulkheadBuilder.enableVirtualThreadsQueueing() #1062

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Oct 22, 2024

This method enables bulkhead queueing for synchronous actions executed on virtual threads. The implementation is a simple two-semaphores bulkhead, which has in fact already existed as FutureThreadPoolBulkhead. This commit generalizes that class and leaves FutureThreadPoolBulkhead as a special case, nearly empty, for guarding asynchronous actions returning Future<V>.

Fixes #987
Closes #1015

This method enables bulkhead queueing for synchronous actions executed
on virtual threads. The implementation is a simple two-semaphores bulkhead,
which has in fact already existed as `FutureThreadPoolBulkhead`. This commit
generalizes that class and leaves `FutureThreadPoolBulkhead` as a special
case, nearly empty, for guarding asynchronous actions returning `Future<V>`.
@Ladicek Ladicek added this to the 6.5.1 milestone Oct 22, 2024
@Ladicek Ladicek merged commit 8bbb6a2 into smallrye:main Oct 22, 2024
13 checks passed
@Ladicek Ladicek deleted the virtual-threads-queueing branch October 22, 2024 14:23
@magicprinc
Copy link
Contributor

I love it!

Only a tiny question of optimization:

Isn't AtomicReference<Thread> executingThread = new AtomicReference<>(Thread.currentThread()); an overkill?

You only read it, so it can be
final Thread executingThread = Thread.currentThread()?

@Ladicek
Copy link
Contributor Author

Ladicek commented Oct 22, 2024

Yeah, that's possible. I might have had some mutations there in the past, who knows 🤷 Removing the AtomicReference should be possible.

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.

Semaphore behavior in Bulkhead strategy (add queueSize and timeout)
2 participants