fix(engine): cleanup orphaned job timeouts and backoffs on migration #13886

lenaschoenburg · 2023-08-14T16:55:19Z

After an update from a previous version, both backoff and deadline column families might contain entries without a corresponding job or multiple entries for a single job.
Before fixing #12797 and #13041, these were cleaned up ad-hoc whenever they were found. This is no longer the case because we now prevent the creation of duplicated entries and always cleanup properly.

This adds two necessary migrations that remove orphaned entries that were left by a previous version. The migrations run once and walk through all deadline and backoff entries, removing those without a job and duplicates which don't match the current job state.

closes #13881

After an update from a previous version, both backoff and deadline column families might contain entries without a corresponding job or multiple entries for a single job. Before fixing #12797 and #13041, these were cleaned up ad-hoc whenever they were found. This is no longer the case because we now prevent the creation of duplicated entries and always cleanup properly. This adds two necessary migrations that remove orphaned entries that were left by a previous version. The migrations run once and walk through all deadline and backoff entries, removing those without a job and duplicates which don't match the current job state.

lenaschoenburg · 2023-08-15T06:48:24Z

engine/src/main/java/io/camunda/zeebe/engine/state/instance/DbJobState.java

+          final var jobKey = key.second().inner();
+          final var deadline = key.first().getValue();
+          final var job = jobsColumnFamily.get(jobKey);
+          if (job == null || job.getRecord().getDeadline() != deadline) {


❓ I'm not entirely sure if this is safe. Could there be a case where the deadline of this job is not in the deadline column family and we would end up deleting all deadlines for this job?

we need this case as there could be multiple entries from the previous behavior and the new behavior only deletes one exact entry, right?

Yes, exactly.

The safest thing to do would be to completely clear the deadline column family and recreate it based on deadlines found for each job.

AFAIK there isn't a case where an activated job wouldn't have an entry in the deadline column family. Every time a job is activated, its deadline and key are added to the deadline column family.

The safest thing to do would be to completely clear the deadline column family and recreate it based on deadlines found for each job.

It's safest, but it's probably more expensive since we would be going through all the available jobs instread of the activated jobs subset.

lenaschoenburg · 2023-08-15T06:48:51Z

engine/src/main/java/io/camunda/zeebe/engine/state/instance/DbJobState.java

+          final var jobKey = key.second().inner();
+          final var backoff = key.first().getValue();
+          final var job = jobsColumnFamily.get(jobKey);
+          if (job == null || job.getRecord().getRetryBackoff() != backoff) {


❓ I'm not entirely sure if this is safe. Could there be a case where the backoff of this job is not in the backoff column family and we would end up deleting all backoffs for this job?

koevskinikola

Nice job @oleschoenburg 🚀

I left some naming-related comments only. Otherwise, the changes look good.

engine/src/main/java/io/camunda/zeebe/engine/state/mutable/MutableJobState.java

engine/src/main/java/io/camunda/zeebe/engine/state/migration/JobBackoffCleanup.java

engine/src/main/java/io/camunda/zeebe/engine/state/migration/JobTimeoutCleanup.java

koevskinikola · 2023-08-15T07:18:06Z

engine/src/main/java/io/camunda/zeebe/engine/state/instance/DbJobState.java

+          final var jobKey = key.second().inner();
+          final var deadline = key.first().getValue();
+          final var job = jobsColumnFamily.get(jobKey);
+          if (job == null || job.getRecord().getDeadline() != deadline) {


AFAIK there isn't a case where an activated job wouldn't have an entry in the deadline column family. Every time a job is activated, its deadline and key are added to the deadline column family.

The safest thing to do would be to completely clear the deadline column family and recreate it based on deadlines found for each job.

It's safest, but it's probably more expensive since we would be going through all the available jobs instread of the activated jobs subset.

koevskinikola

LGTM 🚀

bors merge

remcowesterhoud · 2023-08-15T08:14:31Z

FYI, the backport of this will most likely fail because of changes I made to migrations recently. Let me know if you need help resolving this.

koevskinikola · 2023-08-15T08:31:23Z

Seems like #13537 is popping up again.

zeebe-bors-camunda · 2023-08-15T08:48:10Z

Build succeeded:

Test summary

backport-action · 2023-08-15T08:48:32Z

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-13886-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-13886-to-stable/8.0
git checkout -b backport-13886-to-stable/8.0
ancref=$(git merge-base 416f5cd2bfc0aac1daeb85a31089204b79792db9 dcb132b8e7f1c967dc7df77ed183b05133273dea)
git cherry-pick -x $ancref..dcb132b8e7f1c967dc7df77ed183b05133273dea

backport-action · 2023-08-15T08:48:35Z

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

Please cherry-pick the changes locally.

git fetch origin stable/8.1
git worktree add -d .worktree/backport-13886-to-stable/8.1 origin/stable/8.1
cd .worktree/backport-13886-to-stable/8.1
git checkout -b backport-13886-to-stable/8.1
ancref=$(git merge-base 416f5cd2bfc0aac1daeb85a31089204b79792db9 dcb132b8e7f1c967dc7df77ed183b05133273dea)
git cherry-pick -x $ancref..dcb132b8e7f1c967dc7df77ed183b05133273dea

backport-action · 2023-08-15T08:48:37Z

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

Please cherry-pick the changes locally.

git fetch origin stable/8.2
git worktree add -d .worktree/backport-13886-to-stable/8.2 origin/stable/8.2
cd .worktree/backport-13886-to-stable/8.2
git checkout -b backport-13886-to-stable/8.2
ancref=$(git merge-base 416f5cd2bfc0aac1daeb85a31089204b79792db9 dcb132b8e7f1c967dc7df77ed183b05133273dea)
git cherry-pick -x $ancref..dcb132b8e7f1c967dc7df77ed183b05133273dea

lenaschoenburg · 2023-08-15T08:54:15Z

@remcowesterhoud I was hoping that #13780 would be backported too 😅
The tracking would be really useful here because we want to run the migration only once and we don't have a good indicator on whether it needs to run or not. Do you think you could backport #13780?

remcowesterhoud · 2023-08-15T08:58:55Z

I can give it a go. It's gonna be a manual one though, because I don't want all the commits.

lenaschoenburg · 2023-08-15T13:22:15Z

/backport

backport-action · 2023-08-15T13:22:35Z

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-13886-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-13886-to-stable/8.0
git checkout -b backport-13886-to-stable/8.0
ancref=$(git merge-base 416f5cd2bfc0aac1daeb85a31089204b79792db9 dcb132b8e7f1c967dc7df77ed183b05133273dea)
git cherry-pick -x $ancref..dcb132b8e7f1c967dc7df77ed183b05133273dea

backport-action · 2023-08-15T13:22:37Z

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

Please cherry-pick the changes locally.

git fetch origin stable/8.1
git worktree add -d .worktree/backport-13886-to-stable/8.1 origin/stable/8.1
cd .worktree/backport-13886-to-stable/8.1
git checkout -b backport-13886-to-stable/8.1
ancref=$(git merge-base 416f5cd2bfc0aac1daeb85a31089204b79792db9 dcb132b8e7f1c967dc7df77ed183b05133273dea)
git cherry-pick -x $ancref..dcb132b8e7f1c967dc7df77ed183b05133273dea

backport-action · 2023-08-15T13:22:40Z

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

Please cherry-pick the changes locally.

git fetch origin stable/8.2
git worktree add -d .worktree/backport-13886-to-stable/8.2 origin/stable/8.2
cd .worktree/backport-13886-to-stable/8.2
git checkout -b backport-13886-to-stable/8.2
ancref=$(git merge-base 416f5cd2bfc0aac1daeb85a31089204b79792db9 dcb132b8e7f1c967dc7df77ed183b05133273dea)
git cherry-pick -x $ancref..dcb132b8e7f1c967dc7df77ed183b05133273dea

13909: [Backport stable/8.2] fix(engine): cleanup orphaned job timeouts and backoffs on migration r=megglos a=oleschoenburg Manual backport of #13886 Minor merge conflicts because the list of migrations changed. Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com> Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>

13908: [Backport stable/8.1] fix(engine): cleanup orphaned job timeouts and backoffs on migration r=megglos a=oleschoenburg Manual backport of #13886 Minor merge conflicts because the list of migrations changed. Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com> Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>

13907: [Backport stable/8.0] fix(engine): cleanup orphaned job timeouts and backoffs on migration r=megglos a=oleschoenburg Manual backport of #13886 Minor merge conflicts because the list of migrations changed and because some classes were renamed. Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com> Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>

lenaschoenburg and others added 2 commits August 14, 2023 18:54

test(migration): test orphaned backoff and timeout cleanup

edb6bee

lenaschoenburg marked this pull request as ready for review August 15, 2023 06:45

megglos added backport stable/8.0 backport stable/8.2 Backport a pull request to 8.2.x labels Aug 15, 2023

lenaschoenburg commented Aug 15, 2023

View reviewed changes

koevskinikola requested changes Aug 15, 2023

View reviewed changes

refactor(engine): improve naming of new migrations

dcb132b

lenaschoenburg requested a review from koevskinikola August 15, 2023 07:36

koevskinikola approved these changes Aug 15, 2023

View reviewed changes

zeebe-bors-camunda bot merged commit fe0117e into main Aug 15, 2023
30 of 32 checks passed

zeebe-bors-camunda bot deleted the os/fix-job-entry-leak branch August 15, 2023 08:48

remcowesterhoud mentioned this pull request Oct 4, 2023

Revert storing migrations in MigrationState #14574

Merged

14 tasks

lenaschoenburg mentioned this pull request Nov 29, 2023

Failed jobs are not reactivate after backoff #14329

Closed

sdorokhova mentioned this pull request May 28, 2024

Port importer logic to exporter #18304

Open

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(engine): cleanup orphaned job timeouts and backoffs on migration #13886

fix(engine): cleanup orphaned job timeouts and backoffs on migration #13886

lenaschoenburg commented Aug 14, 2023

lenaschoenburg Aug 15, 2023

megglos Aug 15, 2023 •

edited

lenaschoenburg Aug 15, 2023

lenaschoenburg Aug 15, 2023

koevskinikola Aug 15, 2023

lenaschoenburg Aug 15, 2023

koevskinikola left a comment

koevskinikola Aug 15, 2023

koevskinikola left a comment

remcowesterhoud commented Aug 15, 2023

koevskinikola commented Aug 15, 2023

zeebe-bors-camunda bot commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

lenaschoenburg commented Aug 15, 2023

remcowesterhoud commented Aug 15, 2023 •

edited

lenaschoenburg commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

fix(engine): cleanup orphaned job timeouts and backoffs on migration #13886

fix(engine): cleanup orphaned job timeouts and backoffs on migration #13886

Conversation

lenaschoenburg commented Aug 14, 2023

lenaschoenburg Aug 15, 2023

Choose a reason for hiding this comment

megglos Aug 15, 2023 • edited

Choose a reason for hiding this comment

lenaschoenburg Aug 15, 2023

Choose a reason for hiding this comment

lenaschoenburg Aug 15, 2023

Choose a reason for hiding this comment

koevskinikola Aug 15, 2023

Choose a reason for hiding this comment

lenaschoenburg Aug 15, 2023

Choose a reason for hiding this comment

koevskinikola left a comment

Choose a reason for hiding this comment

koevskinikola Aug 15, 2023

Choose a reason for hiding this comment

koevskinikola left a comment

Choose a reason for hiding this comment

remcowesterhoud commented Aug 15, 2023

koevskinikola commented Aug 15, 2023

zeebe-bors-camunda bot commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

lenaschoenburg commented Aug 15, 2023

remcowesterhoud commented Aug 15, 2023 • edited

lenaschoenburg commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

backport-action commented Aug 15, 2023

megglos Aug 15, 2023 •

edited

remcowesterhoud commented Aug 15, 2023 •

edited