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
Yield actor thread between retries #10844
Conversation
e54e37c
to
3e69937
Compare
Migrates the tests from junit 4 to junit 5 and ensures all strategies are tested, not just two of them, where applicable.
3e69937
to
d2999fd
Compare
As discussed, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 As you said, I don't think we have to ensure BackoffStrategy doesn't interleave tasks. 👍
/** Ensure we use a single thread to better control the scheduling in the tests. */ | ||
@RegisterExtension | ||
private final ControlledActorSchedulerExtension schedulerRule = | ||
new ControlledActorSchedulerExtension( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
bors merge |
10844: Yield actor thread between retries r=npepinpe a=npepinpe ## Description Ensures retry strategies are yielding their thread between retries. Note that this doesn't really apply to the `BackOffRetryStrategy`, as it's already yielding via its usage of timers. This PR also updates the `RetryStrategyTest` to junit 5, and adds test coverage by applying the existing tests to the other strategies where applicable (previously, only the `RecoverableRetryStrategy` and `AbortableRetryStrategy` were tested there). ## Related issues <!-- Which issues are closed by this PR or are related --> closes #10539 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Build failed: |
bors r+ |
10844: Yield actor thread between retries r=npepinpe a=npepinpe ## Description Ensures retry strategies are yielding their thread between retries. Note that this doesn't really apply to the `BackOffRetryStrategy`, as it's already yielding via its usage of timers. This PR also updates the `RetryStrategyTest` to junit 5, and adds test coverage by applying the existing tests to the other strategies where applicable (previously, only the `RecoverableRetryStrategy` and `AbortableRetryStrategy` were tested there). ## Related issues <!-- Which issues are closed by this PR or are related --> closes #10539 Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Build failed: |
:') bors r+ |
Build succeeded: |
Backport failed for Please cherry-pick the changes locally. git fetch origin stable/8.0
git worktree add -d .worktree/backport-10844-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-10844-to-stable/8.0
git checkout -b backport-10844-to-stable/8.0
ancref=$(git merge-base dfae49fafe0f9dd3ba8fbeae1781dab418d5ce1d d2999fd9dd2999ba9610052cebae0c25d59716b4)
git cherry-pick -x $ancref..d2999fd9dd2999ba9610052cebae0c25d59716b4 |
Successfully created backport PR #10866 for |
10869: [Backport stable/8.0] Yield actor thread between retries r=deepthidevaki a=npepinpe # Description Backport of #10844 to `stable/8.1`. relates to #10539 # Conflicts I had to backport the `ControlledActorSchedulerExtension`, mostly, but I think that'll be useful in the future for backporting other things (maybe). There was nothing special for the actual test/PR itself. Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Description
Ensures retry strategies are yielding their thread between retries. Note that this doesn't really apply to the
BackOffRetryStrategy
, as it's already yielding via its usage of timers.This PR also updates the
RetryStrategyTest
to junit 5, and adds test coverage by applying the existing tests to the other strategies where applicable (previously, only theRecoverableRetryStrategy
andAbortableRetryStrategy
were tested there).Related issues
closes #10539
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Please refer to our review guidelines.