Copilot commented on code in PR #13700:
URL: https://github.com/apache/cloudstack/pull/13700#discussion_r3655306025
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -5938,14 +5938,25 @@ public Outcome<VirtualMachine>
removeNicFromVmThroughJobQueue(
final VirtualMachine vm, final Nic nic) {
Long vmId = vm.getId();
String commandName = VmWorkRemoveNicFromVm.class.getName();
- Pair<VmWorkJobVO, Long> pendingWorkJob = retrievePendingWorkJob(vmId,
commandName);
- VmWorkJobVO workJob = pendingWorkJob.first();
+ // The nic uuid must be part of the pending-job lookup key. Without
it, a concurrent request
+ // to remove a different nic from the same vm matches this
still-pending job and joins it
+ // instead of submitting its own, so only one nic is removed while
both callers wait on the
+ // single job and both receive its success. Mirrors the symmetric
addVmToNetworkThroughJobQueue.
+ final List<VmWorkJobVO> pendingWorkJobs =
_workJobDao.listPendingWorkJobs(
+ VirtualMachine.Type.Instance, vmId, commandName,
nic.getUuid());
- if (workJob == null) {
+ VmWorkJobVO workJob;
+ if (pendingWorkJobs != null && pendingWorkJobs.size() > 0) {
+ if (pendingWorkJobs.size() > 1) {
+ throw new CloudRuntimeException(String.format("The number of
jobs to remove nic %s from vm %s are %d", nic.getUuid(), vm.getInstanceName(),
pendingWorkJobs.size()));
Review Comment:
The exception message for the "multiple pending jobs" guard is grammatically
awkward ("number ... are") and a bit unclear about what was found vs. what’s
expected. Consider rephrasing to clearly state the count and that this is an
unexpected/inconsistent state.
--
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]
