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

[Backport 8.0] Cancel job with incident when canceling the process instance #9251

Merged
merged 9 commits into from
Apr 28, 2022

Conversation

korthout
Copy link
Member

Description

Backport of #9219

Related issues

closes #8588

This adds a regression test against activating jobs of cancelled process
instances, by making sure they are cancelled (i.e. deleted).

In this specific case, an uncaught error was thrown on a job which led
to an UNHANDLED_ERROR_EVENT incident. When the process instance is
cancelled, the job was not called, but instead was made activatable
again. This happened, because the incident was resolved as part of the
process instance termination logic. Instead, we should guarantee that
the job is cancelled. A cancelled job can no longer be made
re-activatable. This is a terminal state for jobs, because a cancelled
job is a deleted job.

See #8588 for more details.

(cherry picked from commit d43c344)
This makes it easier to add more values to the predicate, while staying
understandable.

(cherry picked from commit da40a20)
Jobs in the error_thrown state should also be able to be cancelled. By
adding it to this list, we make sure that the job with error_thrown
state can be cancelled by the engine when the element it belongs to is
being terminated.

(cherry picked from commit 57fc9da)
We need to make sure the job is cancelled immediately (synchronously)
because otherwise the IncidentResolvedApplier may still make the job
activatable again.

To cancel the job immediately we simply need to use the statewriter to
write Job Cancelled, instead of using the commandwriter to write Cancel
Job. The statewriter will immediately apply the event when writing it.

(cherry picked from commit 657ca08)
Since the command is no longer written, the test cases that were
verifying that the command was written should be updated to verify that
the job canceled event is written in the same spot.

Note that in the BoundaryEventTest it highlights the difference between
asynchronously canceling the job with a command and synchronously
(immediately) canceling the job with an event.

(cherry picked from commit b9372a5)
The ability to read a metric value will be useful in future Metrics
tests.

(cherry picked from commit 74fe956)
This adds tests that verify that the job metrics are counted when
expected.

Note that the `shouldCountCanceled` case fails, because canceling the
job was no longer counted in the metrics.

(cherry picked from commit d9b3413)
This behavior was broken in an earlier commit, when we switched from
writing a Cancel Job command to a Job Canceled event.

(cherry picked from commit 0aac0d7)
It's important to highlight duplication, because if either changes, the
other should be changed as well.

(cherry picked from commit 25fae05)
@korthout
Copy link
Member Author

@pihme Please have a look, no need for a full review as the cherry-picking was without conflicts. The automatic backporting had failed because the other one had already failed. IMO, this just needs an approval and we can merge it

@korthout korthout requested a review from pihme April 28, 2022 15:04
@korthout
Copy link
Member Author

bors merge

zeebe-bors-camunda bot added a commit that referenced this pull request Apr 28, 2022
9251: [Backport 8.0] Cancel job with incident when canceling the process instance r=korthout a=korthout

## Description

<!-- Link to the PR that is back ported -->

Backport of #9219

## Related issues

<!-- Link to the related issues of the origin PR -->

closes #8588 


Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
@zeebe-bors-camunda
Copy link
Contributor

Build failed:

@korthout
Copy link
Member Author

Forking failed.

bors retry

@zeebe-bors-camunda
Copy link
Contributor

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit 3931a89 into stable/8.0 Apr 28, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the backport-9219-to-stable/8.0 branch April 28, 2022 17:25
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.

None yet

2 participants