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

Remove ActorControl#runUntilDone #9850

Merged
merged 9 commits into from
Jul 25, 2022
Merged

Remove ActorControl#runUntilDone #9850

merged 9 commits into from
Jul 25, 2022

Conversation

npepinpe
Copy link
Member

Description

This PR removes the dangerous ActorControl#runUntilDone and replaces its usage with simple re-scheduling via run or submit or runDelayed where appropriate. At the same time, since we don't need it anymore, it also removes ActorControl#done. We keep ActorControl#yieldThread for jobs triggered by subscriptions/conditions (e.g. dispatcher subscriptions). This means only such jobs are not auto completing anymore (but they never had to call done).

Related issues

related to #8302

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually
  • The change has been verified by a QA run
  • The impact of the changes is verified by a benchmark

Documentation:

  • The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
  • New content is added to the release announcement
  • If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

Please refer to our review guidelines.

Replaces the usage of `runUntilDone` in the retry mechanisms with a
control return value. The old `yieldThread` is replaced with a recursive
submit, and the old done simply does nothing. To trigger a retry, the
mechanism simply has to return `Control.RETRY`.
Removes `ActorControl#runUntilDone` to avoid endless loops. This also
inlines that any job not triggered by a subscription is always auto
completing, and removes `ActorControl#done()`. We keep
`ActorControl#yieldThread` for subscriptions/conditions for now, but
these should be the only ones using it.
@github-actions
Copy link
Contributor

github-actions bot commented Jul 20, 2022

Unit Test Results

   751 files   -   41     751 suites   - 41   26m 41s ⏱️ - 1h 12m 5s
6 173 tests +318  6 163 ✔️ +317  10 💤 +1  0 ±0 
6 340 runs  +316  6 330 ✔️ +315  10 💤 +1  0 ±0 

Results for commit df99b94. ± Comparison against base commit bbbb0d3.

♻️ This comment has been updated with latest results.

@npepinpe
Copy link
Member Author

The one failing test is a known flaky test.

Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

This is great 😍

I only have one suggestion to remove isAutoCompleting, everything else LGTM 👍

@npepinpe
Copy link
Member Author

bors merge

zeebe-bors-camunda bot added a commit that referenced this pull request Jul 25, 2022
9850: Remove ActorControl#runUntilDone r=npepinpe a=npepinpe

## Description

This PR removes the dangerous `ActorControl#runUntilDone` and replaces its usage with simple re-scheduling via `run` or `submit` or `runDelayed` where appropriate. At the same time, since we don't need it anymore, it also removes `ActorControl#done`. We keep `ActorControl#yieldThread` for jobs triggered by subscriptions/conditions (e.g. dispatcher subscriptions). This means only such jobs are not auto completing anymore (but they never had to call done).

## Related issues

related to #8302



Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
@zeebe-bors-camunda
Copy link
Contributor

Build failed:

@npepinpe
Copy link
Member Author

bors r+

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit 6cc54ae into main Jul 25, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the 8302-runUntilDone branch July 25, 2022 09:24
@npepinpe
Copy link
Member Author

/backport

@backport-action
Copy link
Collaborator

Backport failed for stable/1.3, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/1.3
git worktree add -d .worktree/backport-9850-to-stable/1.3 origin/stable/1.3
cd .worktree/backport-9850-to-stable/1.3
git checkout -b backport-9850-to-stable/1.3
ancref=$(git merge-base bbbb0d3e8455efa051edde4a20e3152bc5e3636d df99b9451738dd9659b55b73b60fcf6ad96ebeaa)
git cherry-pick -x $ancref..df99b9451738dd9659b55b73b60fcf6ad96ebeaa

@backport-action
Copy link
Collaborator

Backport failed for stable/8.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/8.0
git worktree add -d .worktree/backport-9850-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-9850-to-stable/8.0
git checkout -b backport-9850-to-stable/8.0
ancref=$(git merge-base bbbb0d3e8455efa051edde4a20e3152bc5e3636d df99b9451738dd9659b55b73b60fcf6ad96ebeaa)
git cherry-pick -x $ancref..df99b9451738dd9659b55b73b60fcf6ad96ebeaa

zeebe-bors-camunda bot added a commit that referenced this pull request Jul 25, 2022
9875: [Backport stable/8.0] Remove ActorControl#runUntilDone r=npepinpe a=npepinpe

## Description

This PR backports #9850 to stable/8.0. There was one additional commit not in the PR which had to be backported/adapted, b052712. This is the last commit of the PR.

## Related issues

backports #9850 
related to #8302 



Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
zeebe-bors-camunda bot added a commit that referenced this pull request Jul 25, 2022
9876: [Backport stable/1.3] Remove ActorControl#runUntilDone r=npepinpe a=npepinpe

## Description

This PR backports #9850 to stable/8.0. There was one additional commit not in the PR which had to be backported/adapted, b052712. This is the last commit of the PR.

## Related issues

backports #9850 
related to #8302 



Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
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.

None yet

3 participants