Skip to content

Commit 2e60b77

Browse files
harikrishna-patnalaharikrishna-patnala
committed
Submitting multiple dynamic VM Scaling API commands for the same instance can result in two usage events in the same second causing a compound key violation in usage service
Root cause: Even though dynamic scaling job is handled in vmworkjob queue which ensures serilizing multiple jobs but the database updating and generating usage events are out of the job queue. Solution: Moved all updations into the job queue
1 parent 2e3390f commit 2e60b77

7 files changed

Lines changed: 63 additions & 87 deletions

File tree

‎engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ void advanceReboot(String vmUuid, Map<VirtualMachineProfile.Param, Object> param
152152
* @param serviceOfferingId
153153
* @return
154154
*/
155-
boolean upgradeVmDb(long vmId, long serviceOfferingId);
155+
boolean upgradeVmDb(long vmId, ServiceOffering newServiceOffering, ServiceOffering currentServiceOffering);
156156

157157
/**
158158
* @param vm

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
import com.cloud.agent.api.PrepareForMigrationAnswer;
4343
import com.cloud.agent.api.to.DpdkTO;
44+
import com.cloud.event.UsageEventVO;
4445
import com.cloud.offering.NetworkOffering;
4546
import com.cloud.offerings.dao.NetworkOfferingDetailsDao;
4647
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
@@ -232,6 +233,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
232233

233234
private static final String VM_SYNC_ALERT_SUBJECT = "VM state sync alert";
234235

236+
@Inject
237+
private UserVmManager _userVmMgr;
235238
@Inject
236239
private DataStoreManager dataStoreMgr;
237240
@Inject
@@ -3406,13 +3409,20 @@ public void checkIfCanUpgrade(final VirtualMachine vmInstance, final ServiceOffe
34063409
}
34073410

34083411
@Override
3409-
public boolean upgradeVmDb(final long vmId, final long serviceOfferingId) {
3410-
final VMInstanceVO vmForUpdate = _vmDao.createForUpdate();
3411-
vmForUpdate.setServiceOfferingId(serviceOfferingId);
3412-
final ServiceOffering newSvcOff = _entityMgr.findById(ServiceOffering.class, serviceOfferingId);
3412+
public boolean upgradeVmDb(final long vmId, final ServiceOffering newServiceOffering, ServiceOffering currentServiceOffering) {
3413+
3414+
final VMInstanceVO vmForUpdate = _vmDao.findById(vmId);
3415+
vmForUpdate.setServiceOfferingId(newServiceOffering.getId());
3416+
final ServiceOffering newSvcOff = _entityMgr.findById(ServiceOffering.class, newServiceOffering.getId());
34133417
vmForUpdate.setHaEnabled(newSvcOff.isOfferHA());
34143418
vmForUpdate.setLimitCpuUse(newSvcOff.getLimitCpuUse());
34153419
vmForUpdate.setServiceOfferingId(newSvcOff.getId());
3420+
if (newServiceOffering.isDynamic()) {
3421+
saveCustomOfferingDetails(vmId, newServiceOffering);
3422+
}
3423+
if (currentServiceOffering.isDynamic() && !newServiceOffering.isDynamic()) {
3424+
removeCustomOfferingDetails(vmId);
3425+
}
34163426
return _vmDao.update(vmId, vmForUpdate);
34173427
}
34183428

@@ -4177,7 +4187,10 @@ private VMInstanceVO orchestrateReConfigureVm(final String vmUuid, final Service
41774187
s_logger.error("Unable to scale vm due to " + (reconfigureAnswer == null ? "" : reconfigureAnswer.getDetails()));
41784188
throw new CloudRuntimeException("Unable to scale vm due to " + (reconfigureAnswer == null ? "" : reconfigureAnswer.getDetails()));
41794189
}
4180-
4190+
upgradeVmDb(vm.getId(), newServiceOffering, oldServiceOffering);
4191+
if (vm.getType().equals(VirtualMachine.Type.User)) {
4192+
_userVmMgr.generateUsageEvent(vm, vm.isDisplayVm(), EventTypes.EVENT_VM_DYNAMIC_SCALE);
4193+
}
41814194
success = true;
41824195
} catch (final OperationTimedoutException e) {
41834196
throw new AgentUnavailableException("Operation timed out on reconfiguring " + vm, dstHostId);
@@ -4186,7 +4199,7 @@ private VMInstanceVO orchestrateReConfigureVm(final String vmUuid, final Service
41864199
} finally {
41874200
if (!success) {
41884201
_capacityMgr.releaseVmCapacity(vm, false, false, vm.getHostId()); // release the new capacity
4189-
vm.setServiceOfferingId(oldServiceOffering.getId());
4202+
upgradeVmDb(vm.getId(), newServiceOffering, oldServiceOffering); // rollback
41904203
_capacityMgr.allocateVmCapacity(vm, false); // allocate the old capacity
41914204
}
41924205
}
@@ -4195,6 +4208,33 @@ private VMInstanceVO orchestrateReConfigureVm(final String vmUuid, final Service
41954208

41964209
}
41974210

4211+
private void removeCustomOfferingDetails(long vmId) {
4212+
Map<String, String> details = userVmDetailsDao.listDetailsKeyPairs(vmId);
4213+
details.remove(UsageEventVO.DynamicParameters.cpuNumber.name());
4214+
details.remove(UsageEventVO.DynamicParameters.cpuSpeed.name());
4215+
details.remove(UsageEventVO.DynamicParameters.memory.name());
4216+
List<UserVmDetailVO> detailList = new ArrayList<UserVmDetailVO>();
4217+
for(Map.Entry<String, String> entry: details.entrySet()) {
4218+
UserVmDetailVO detailVO = new UserVmDetailVO(vmId, entry.getKey(), entry.getValue(), true);
4219+
detailList.add(detailVO);
4220+
}
4221+
userVmDetailsDao.saveDetails(detailList);
4222+
}
4223+
4224+
private void saveCustomOfferingDetails(long vmId, ServiceOffering serviceOffering) {
4225+
//save the custom values to the database.
4226+
Map<String, String> details = userVmDetailsDao.listDetailsKeyPairs(vmId);
4227+
details.put(UsageEventVO.DynamicParameters.cpuNumber.name(), serviceOffering.getCpu().toString());
4228+
details.put(UsageEventVO.DynamicParameters.cpuSpeed.name(), serviceOffering.getSpeed().toString());
4229+
details.put(UsageEventVO.DynamicParameters.memory.name(), serviceOffering.getRamSize().toString());
4230+
List<UserVmDetailVO> detailList = new ArrayList<UserVmDetailVO>();
4231+
for (Map.Entry<String, String> entry: details.entrySet()) {
4232+
UserVmDetailVO detailVO = new UserVmDetailVO(vmId, entry.getKey(), entry.getValue(), true);
4233+
detailList.add(detailVO);
4234+
}
4235+
userVmDetailsDao.saveDetails(detailList);
4236+
}
4237+
41984238
@Override
41994239
public String getConfigComponentName() {
42004240
return VirtualMachineManager.class.getSimpleName();

‎server/src/main/java/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4086,15 +4086,7 @@ private VirtualMachine upgradeStoppedSystemVm(final Long systemVmId, final Long
40864086
}
40874087
_itMgr.checkIfCanUpgrade(systemVm, newServiceOffering);
40884088

4089-
final boolean result = _itMgr.upgradeVmDb(systemVmId, serviceOfferingId);
4090-
4091-
if (newServiceOffering.isDynamic()) {
4092-
//save the custom values to the database.
4093-
_userVmMgr.saveCustomOfferingDetails(systemVmId, newServiceOffering);
4094-
}
4095-
if (currentServiceOffering.isDynamic() && !newServiceOffering.isDynamic()) {
4096-
_userVmMgr.removeCustomOfferingDetails(systemVmId);
4097-
}
4089+
final boolean result = _itMgr.upgradeVmDb(systemVmId, newServiceOffering, currentServiceOffering);
40984090

40994091
if (result) {
41004092
return _vmInstanceDao.findById(systemVmId);

‎server/src/main/java/com/cloud/vm/UserVmManager.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.cloud.exception.ManagementServerException;
3333
import com.cloud.exception.ResourceUnavailableException;
3434
import com.cloud.exception.VirtualMachineMigrationException;
35-
import com.cloud.offering.ServiceOffering;
3635
import com.cloud.service.ServiceOfferingVO;
3736
import com.cloud.storage.Storage.StoragePoolType;
3837
import com.cloud.user.Account;
@@ -117,10 +116,6 @@ UserVm updateVirtualMachine(long id, String displayName, String group, Boolean h
117116
//find a common place for all the scaling and upgrading code of both user and systemvms.
118117
void validateCustomParameters(ServiceOfferingVO serviceOffering, Map<String, String> customParameters);
119118

120-
public void saveCustomOfferingDetails(long vmId, ServiceOffering serviceOffering);
121-
122-
public void removeCustomOfferingDetails(long vmId);
123-
124119
void generateUsageEvent(VirtualMachine vm, boolean isDisplay, String eventType);
125120

126121
void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString);

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 2 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,14 +1007,7 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
10071007
// Check that the specified service offering ID is valid
10081008
_itMgr.checkIfCanUpgrade(vmInstance, newServiceOffering);
10091009

1010-
_itMgr.upgradeVmDb(vmId, svcOffId);
1011-
if (newServiceOffering.isDynamic()) {
1012-
//save the custom values to the database.
1013-
saveCustomOfferingDetails(vmId, newServiceOffering);
1014-
}
1015-
if (currentServiceOffering.isDynamic() && !newServiceOffering.isDynamic()) {
1016-
removeCustomOfferingDetails(vmId);
1017-
}
1010+
_itMgr.upgradeVmDb(vmId, newServiceOffering, currentServiceOffering);
10181011

10191012
// Increment or decrement CPU and Memory count accordingly.
10201013
if (newCpu > currentCpu) {
@@ -1132,14 +1125,7 @@ private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map<String
11321125
ServiceOffering newSvcOffering = _offeringDao.findById(svcOffId);
11331126
_accountMgr.checkAccess(owner, newSvcOffering, _dcDao.findById(vmInstance.getDataCenterId()));
11341127

1135-
_itMgr.upgradeVmDb(vmId, svcOffId);
1136-
if (newServiceOffering.isDynamic()) {
1137-
//save the custom values to the database.
1138-
saveCustomOfferingDetails(vmId, newServiceOffering);
1139-
}
1140-
if (currentServiceOffering.isDynamic() && !newServiceOffering.isDynamic()) {
1141-
removeCustomOfferingDetails(vmId);
1142-
}
1128+
_itMgr.upgradeVmDb(vmId, newServiceOffering, currentServiceOffering);
11431129

11441130
// Increment or decrement CPU and Memory count accordingly.
11451131
if (newCpu > currentCpu) {
@@ -1659,10 +1645,6 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx
16591645
// Generate usage event for VM upgrade
16601646
generateUsageEvent(vmInstance, vmInstance.isDisplayVm(), EventTypes.EVENT_VM_UPGRADE);
16611647
}
1662-
if (vmInstance.getState().equals(State.Running)) {
1663-
// Generate usage event for Dynamic scaling of VM
1664-
generateUsageEvent( vmInstance, vmInstance.isDisplayVm(), EventTypes.EVENT_VM_DYNAMIC_SCALE);
1665-
}
16661648
return vmInstance;
16671649
} else {
16681650
throw new CloudRuntimeException("Failed to scale the VM");
@@ -1835,17 +1817,9 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI
18351817
}
18361818

18371819
// #3 scale the vm now
1838-
_itMgr.upgradeVmDb(vmId, newServiceOfferingId);
1839-
if (newServiceOffering.isDynamic()) {
1840-
//save the custom values to the database.
1841-
saveCustomOfferingDetails(vmId, newServiceOffering);
1842-
}
18431820
vmInstance = _vmInstanceDao.findById(vmId);
18441821
_itMgr.reConfigureVm(vmInstance.getUuid(), currentServiceOffering, existingHostHasCapacity);
18451822
success = true;
1846-
if (currentServiceOffering.isDynamic() && !newServiceOffering.isDynamic()) {
1847-
removeCustomOfferingDetails(vmId);
1848-
}
18491823
return success;
18501824
} catch (InsufficientCapacityException e) {
18511825
s_logger.warn("Received exception while scaling ", e);
@@ -1857,16 +1831,10 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI
18571831
s_logger.warn("Received exception while scaling ", e);
18581832
} finally {
18591833
if (!success) {
1860-
_itMgr.upgradeVmDb(vmId, currentServiceOffering.getId()); // rollback
1861-
18621834
// Decrement CPU and Memory count accordingly.
18631835
if (newCpu > currentCpu) {
18641836
_resourceLimitMgr.decrementResourceCount(caller.getAccountId(), ResourceType.cpu, new Long(newCpu - currentCpu));
18651837
}
1866-
//restoring old service offering will take care of removing new SO.
1867-
if(currentServiceOffering.isDynamic()){
1868-
saveCustomOfferingDetails(vmId, currentServiceOffering);
1869-
}
18701838

18711839
if (memoryDiff > 0) {
18721840
_resourceLimitMgr.decrementResourceCount(caller.getAccountId(), ResourceType.memory, new Long(memoryDiff));
@@ -1878,35 +1846,6 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI
18781846
return success;
18791847
}
18801848

1881-
@Override
1882-
public void saveCustomOfferingDetails(long vmId, ServiceOffering serviceOffering) {
1883-
//save the custom values to the database.
1884-
Map<String, String> details = userVmDetailsDao.listDetailsKeyPairs(vmId);
1885-
details.put(UsageEventVO.DynamicParameters.cpuNumber.name(), serviceOffering.getCpu().toString());
1886-
details.put(UsageEventVO.DynamicParameters.cpuSpeed.name(), serviceOffering.getSpeed().toString());
1887-
details.put(UsageEventVO.DynamicParameters.memory.name(), serviceOffering.getRamSize().toString());
1888-
List<UserVmDetailVO> detailList = new ArrayList<UserVmDetailVO>();
1889-
for (Map.Entry<String, String> entry: details.entrySet()) {
1890-
UserVmDetailVO detailVO = new UserVmDetailVO(vmId, entry.getKey(), entry.getValue(), true);
1891-
detailList.add(detailVO);
1892-
}
1893-
userVmDetailsDao.saveDetails(detailList);
1894-
}
1895-
1896-
@Override
1897-
public void removeCustomOfferingDetails(long vmId) {
1898-
Map<String, String> details = userVmDetailsDao.listDetailsKeyPairs(vmId);
1899-
details.remove(UsageEventVO.DynamicParameters.cpuNumber.name());
1900-
details.remove(UsageEventVO.DynamicParameters.cpuSpeed.name());
1901-
details.remove(UsageEventVO.DynamicParameters.memory.name());
1902-
List<UserVmDetailVO> detailList = new ArrayList<UserVmDetailVO>();
1903-
for(Map.Entry<String, String> entry: details.entrySet()) {
1904-
UserVmDetailVO detailVO = new UserVmDetailVO(vmId, entry.getKey(), entry.getValue(), true);
1905-
detailList.add(detailVO);
1906-
}
1907-
userVmDetailsDao.saveDetails(detailList);
1908-
}
1909-
19101849
@Override
19111850
public HashMap<Long, VmStatsEntry> getVirtualMachineStatistics(long hostId, String hostName, List<Long> vmIds) throws CloudRuntimeException {
19121851
HashMap<Long, VmStatsEntry> vmStatsById = new HashMap<Long, VmStatsEntry>();

‎server/src/test/java/com/cloud/vm/UserVmManagerTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ public void testScaleVMF3() throws Exception {
572572
doReturn(VirtualMachine.State.Stopped).when(_vmInstance).getState();
573573
when(_vmDao.findById(anyLong())).thenReturn(null);
574574

575-
doReturn(true).when(_itMgr).upgradeVmDb(anyLong(), anyLong());
575+
doReturn(true).when(_itMgr).upgradeVmDb(anyLong(), so1, so2);
576576

577577
//when(_vmDao.findById(anyLong())).thenReturn(_vmMock);
578578

@@ -621,7 +621,7 @@ public void testScaleVMF4() throws Exception {
621621
when(_capacityMgr.checkIfHostHasCapacity(anyLong(), anyInt(), anyLong(), anyBoolean(), anyFloat(), anyFloat(), anyBoolean())).thenReturn(false);
622622
when(_itMgr.reConfigureVm(_vmInstance.getUuid(), so1, false)).thenReturn(_vmInstance);
623623

624-
doReturn(true).when(_itMgr).upgradeVmDb(anyLong(), anyLong());
624+
doReturn(true).when(_itMgr).upgradeVmDb(anyLong(), so1, so2);
625625

626626
when(_vmDao.findById(anyLong())).thenReturn(_vmMock);
627627

‎usage/src/main/java/com/cloud/usage/UsageManagerImpl.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1199,12 +1199,22 @@ private void createVMHelperEvent(UsageEventVO event) {
11991199
new UsageVMInstanceVO(UsageTypes.ALLOCATED_VM, zoneId, event.getAccountId(), vmId, vmName, soId, templateId, hypervisorType, event.getCreateDate(), null);
12001200
populateDynamicComputeOfferingDetailsAndPersist(usageInstanceNew, event.getId());
12011201
} else if (EventTypes.EVENT_VM_DYNAMIC_SCALE.equals(event.getType())) {
1202+
// Looking for any duplicate usage entries which are recorded at the same time
1203+
SearchCriteria<UsageVMInstanceVO> sc = _usageInstanceDao.createSearchCriteria();
1204+
sc.addAnd("vmInstanceId", SearchCriteria.Op.EQ, Long.valueOf(vmId));
1205+
sc.addAnd("startDate", SearchCriteria.Op.EQ, event.getCreateDate());
1206+
sc.addAnd("usageType", SearchCriteria.Op.EQ, UsageTypes.RUNNING_VM);
1207+
List<UsageVMInstanceVO> usageInstances = _usageInstanceDao.search(sc, null);
1208+
if (usageInstances != null) {
1209+
s_logger.warn("Found duplicate dynamic scaling entries at the same timespamp: " + event.getCreateDate() + ". skipping this event");
1210+
return;
1211+
}
12021212
// Ending the running vm event
12031213
SearchCriteria<UsageVMInstanceVO> sc = _usageInstanceDao.createSearchCriteria();
12041214
sc.addAnd("vmInstanceId", SearchCriteria.Op.EQ, Long.valueOf(vmId));
12051215
sc.addAnd("endDate", SearchCriteria.Op.NULL);
12061216
sc.addAnd("usageType", SearchCriteria.Op.EQ, UsageTypes.RUNNING_VM);
1207-
List<UsageVMInstanceVO> usageInstances = _usageInstanceDao.search(sc, null);
1217+
usageInstances = _usageInstanceDao.search(sc, null);
12081218
if (usageInstances != null) {
12091219
if (usageInstances.size() > 1) {
12101220
s_logger.warn("found multiple entries for a vm running with id: " + vmId + ", ending them all...");

0 commit comments

Comments
 (0)