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

bulkhead event loop integration #419

Merged
merged 5 commits into from
May 17, 2021

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented May 14, 2021

Resolves #404

@Ladicek Ladicek added this to the 5.1.0 milestone May 14, 2021
@Ladicek Ladicek force-pushed the bulkhead-event-loop-integration branch from 01670f7 to 4220366 Compare May 14, 2021 13:49
This commit includes a significant refactoring of the event loop
integration API. This would normally be a breaking change, but
the event loop integration API is private, and isn't yet present
in any release, so it's OK.

The event loop is now required to provide a `Scheduler` (instead of
_being_ a `Scheduler`), as well as an `Executor`. The `Scheduler`
is used for retries, while the `Executor` is used for running
queued bulkhead tasks.
This commit removes the `MainScheduler` class, which used to check
the current thread right before scheduling the invocation. Instead,
we now remember the event loop scheduler eagerly, if the invocation
thread is an event loop thread, and later just use it. This is close
to how an event loop executor is remembered for non-blocking method
invocations in `CompletionStageExecution`.

At the same time, the `VertxScheduler` implementation is improved
to always schedule tasks on the correct event loop thread, in case
there are multiple.
@Ladicek Ladicek force-pushed the bulkhead-event-loop-integration branch from 4220366 to 93877f8 Compare May 14, 2021 14:11
@Ladicek Ladicek requested a review from a team as a code owner May 14, 2021 14:55
With this commit, we no longer use an event loop for task scheduling.
Instead, we always use our `Timer` for scheduling, and only use
an event loop for task execution (using `Executor`). If we have to
schedule a task for execution on an event loop, we create another task
that executes the original on an event loop, and schedule that.

The primary motivation is to avoid having to implement a scheduler
on top of an event loop if we already have a decent one, but there's
another motive as well: the Vert.x timer, which we used to implement
the Vert.x scheduler, is rather inaccurate.
@Ladicek Ladicek merged commit e086cd9 into smallrye:main May 17, 2021
@Ladicek Ladicek deleted the bulkhead-event-loop-integration branch May 17, 2021 13:32
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.

integrate thread pool style bulkhead with event loop
1 participant