Skip to content

framework: do not double-execute an async job after returning its queue item - #14039

Open
nagaboinaramgopal wants to merge 1 commit into
apache:4.20from
nagaboinaramgopal:fix/asyncjob-double-execution
Open

framework: do not double-execute an async job after returning its queue item#14039
nagaboinaramgopal wants to merge 1 commit into
apache:4.20from
nagaboinaramgopal:fix/asyncjob-double-execution

Conversation

@nagaboinaramgopal

Copy link
Copy Markdown

Description

In executeQueueItem, when persisting the executing management-server id fails
(the DB-deadlock case the catch block exists for), the queue item is returned to
the queue so it can be retried later. Execution then fell through to
scheduleExecution(job), so the job was dispatched now AND re-dequeued and
dispatched again by the heartbeat, running the same job (VM start/deploy, volume
create, snapshot, etc.) twice concurrently and defeating the sync queue's
serialization.

Return after returning the item so the job runs once, on the retry.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • Minor

How Has This Been Tested?

Added a unit test that makes the executing-msid update throw and asserts the
queue item is returned and the job is not also scheduled. Also built the standard
packages and deployed on a KVM advanced zone.

@nagaboinaramgopal
nagaboinaramgopal force-pushed the fix/asyncjob-double-execution branch from c397396 to 9259d8b Compare September 2, 2026 23:22
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.78%. Comparing base (2cd8c5e) to head (8c06db5).

Files with missing lines Patch % Lines
...stack/framework/jobs/impl/AsyncJobManagerImpl.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               4.20   #14039      +/-   ##
============================================
+ Coverage     16.34%   19.78%   +3.44%     
- Complexity    13574    19993    +6419     
============================================
  Files          5669     6371     +702     
  Lines        501368   575900   +74532     
  Branches      60903    70495    +9592     
============================================
+ Hits          81964   113970   +32006     
- Misses       410219   449500   +39281     
- Partials       9185    12430    +3245     
Flag Coverage Δ
uitests 3.53% <ø> (-0.62%) ⬇️
unittests 21.06% <0.00%> (+3.85%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ue item

In executeQueueItem, when persisting the executing MS id fails (the DB-deadlock
case the catch block exists for), the queue item is returned to the queue so it
can be retried. Execution then fell through to scheduleExecution(job), so the job
was dispatched now AND re-dequeued and dispatched again by the heartbeat, running
the same job (VM start/deploy, volume create, snapshot, etc.) twice concurrently
and defeating the sync queue's serialization. Return after returning the item.
@nagaboinaramgopal
nagaboinaramgopal force-pushed the fix/asyncjob-double-execution branch from 9259d8b to 8c06db5 Compare September 3, 2026 17:18
@nagaboinaramgopal
nagaboinaramgopal changed the base branch from main to 4.20 September 3, 2026 17:54
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19093

@DaanHoogland

Copy link
Copy Markdown
Contributor

@nagaboinaramgopal the method you are touching has two consecutive try blocks, both of which have nested try block. If you have time, do you feel like cleaning up the code? (does not need to be in the scope of this PR)

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants