nagaboinaramgopal commented on PR #14039:
URL: https://github.com/apache/cloudstack/pull/14039#issuecomment-5572213709
> @nagaboinaramgopal do you have reproduction steps for the issue PR is
fixing?
Thanks @shwstppr . No linked issue, it came from reading `executeQueueItem`,
and it reproduces deterministically as a fail-before / pass-after test.
The scenario is the DB deadlock path that method already documents in its
own TODO comment:
1. A queued async job is dequeued and `executeQueueItem` tries
`_jobDao.update(job.getId(), job)` to stamp the executing management-server id.
2. That update throws, most commonly the DB deadlock the TODO is written to
survive.
3. The catch returns the item to the sync queue with
`_queueMgr.returnItem(...)`, so the job will be picked up and run on a later
turn.
4. Before this change, execution then fell through into the next `try {
scheduleExecution(job); }` block and scheduled the same job to run now as well.
So the job is both requeued for later and executed now: it runs twice.
The fix returns right after the item is put back on the queue, so a failed
dispatch only requeues and does not also execute.
`AsyncJobManagerImplExecuteQueueItemTest.executeQueueItemDoesNotScheduleWhenTheJobUpdateFailsAndItemIsReturned`
reproduces it: it makes the job update throw and verifies the item is returned
to the queue and `scheduleExecution` is not called.
On the unpatched code the test fails, which is the double execution:
executeQueueItemDoesNotScheduleWhenTheJobUpdateFailsAndItemIsReturned
<<< FAILURE!
asyncJobManager.scheduleExecution(...) was invoked
Tests run: 1, Failures: 1
With the change it passes:
Tests run: 1, Failures: 0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
