Skip to content

Drain per-host reservation when a VM starts on a different host #13363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Kukunin wants to merge 1 commit into
base: main
Choose a base branch
from
+81 −45
Open

Drain per-host reservation when a VM starts on a different host #13363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ public interface CapacityManager {
"'<vcpu />' element of domain XMLs. If it is set to a value less than or equal to '0', then the host's CPU cores capacity will be considered.",
true, ConfigKey.Scope.Cluster);

public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReservered, Long hostId);
public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReserved, Long hostId);

void allocateVmCapacity(VirtualMachine vm, boolean fromLastHost);
void allocateVmCapacity(VirtualMachine vm);

/**
* @param hostId Id of the host to check capacity
Expand Down
2 changes: 1 addition & 1 deletion engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5241,7 +5241,7 @@ private VMInstanceVO orchestrateReConfigureVm(String vmUuid, ServiceOffering old
}

if (reconfiguringOnExistingHost) {
_capacityMgr.allocateVmCapacity(vm, false);
_capacityMgr.allocateVmCapacity(vm);
}

} catch (final OperationTimedoutException e) {
Expand Down
65 changes: 23 additions & 42 deletions server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,19 +167,19 @@ public boolean start() {

@DB
@Override
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReservered, final Long hostId) {
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReserved, final Long hostId) {
if (hostId == null) {
return true;
}
HostVO host = _hostDao.findById(hostId);
if (HypervisorType.External.equals(host.getHypervisorType())) {
return true;
}
return releaseVmCapacity(vm, moveFromReserved, moveToReservered, host);
return releaseVmCapacity(vm, moveFromReserved, moveToReserved, host);
}

@DB
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReservered, final Host host) {
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReserved, final Host host) {
if (host == null) {
return true;
}
Expand Down Expand Up @@ -241,7 +241,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
capacityCpuCore.setUsedCapacity(usedCpuCore - vmCPUCore);
}

if (moveToReservered) {
if (moveToReserved) {
if (reservedCpu + vmCPU <= totalCpu) {
capacityCpu.setReservedCapacity(reservedCpu + vmCPU);
}
Expand All @@ -264,11 +264,11 @@ public void doInTransactionWithoutResult(TransactionStatus status) {

logger.debug("release cpu from host: {}, old used: {}, " +
"reserved: {}, actual total: {}, total with overprovisioning: {}; " +
"new used: {},reserved:{}; movedfromreserved: {},moveToReservered: {}", host, usedCpu, reservedCpu, actualTotalCpu, totalCpu, capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), moveFromReserved, moveToReservered);
"new used: {},reserved:{}; movedfromreserved: {},moveToReserved: {}", host, usedCpu, reservedCpu, actualTotalCpu, totalCpu, capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), moveFromReserved, moveToReserved);

logger.debug("release mem from host: {}, old used: {}, " +
"reserved: {}, total: {}; new used: {}, reserved: {}; " +
"movedfromreserved: {}, moveToReservered: {}", host, toHumanReadableSize(usedMem), toHumanReadableSize(reservedMem), toHumanReadableSize(totalMem), toHumanReadableSize(capacityMemory.getUsedCapacity()), toHumanReadableSize(capacityMemory.getReservedCapacity()), moveFromReserved, moveToReservered);
"movedfromreserved: {}, moveToReserved: {}", host, toHumanReadableSize(usedMem), toHumanReadableSize(reservedMem), toHumanReadableSize(totalMem), toHumanReadableSize(capacityMemory.getUsedCapacity()), toHumanReadableSize(capacityMemory.getReservedCapacity()), moveFromReserved, moveToReserved);

_capacityDao.update(capacityCpu.getId(), capacityCpu);
_capacityDao.update(capacityMemory.getId(), capacityMemory);
Expand All @@ -285,7 +285,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {

@DB
@Override
public void allocateVmCapacity(VirtualMachine vm, final boolean fromLastHost) {
public void allocateVmCapacity(VirtualMachine vm) {

final long hostId = vm.getHostId();
final HostVO host = _hostDao.findById(hostId);
Expand Down Expand Up @@ -328,7 +328,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
long usedCpuCore = capacityCpuCore.getUsedCapacity();
long reservedCpu = capacityCpu.getReservedCapacity();
long reservedMem = capacityMem.getReservedCapacity();
long reservedCpuCore = capacityCpuCore.getReservedCapacity();
long actualTotalCpu = capacityCpu.getTotalCapacity();
long actualTotalMem = capacityMem.getTotalCapacity();
long totalCpu = (long)(actualTotalCpu * cpuOvercommitRatio);
Expand All @@ -349,41 +348,25 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
capacityMem.setUsedCapacity(usedMem + ram);
capacityCpuCore.setUsedCapacity(usedCpuCore + cpucore);

if (fromLastHost) {
/* alloc from reserved */
if (logger.isDebugEnabled()) {
logger.debug("We are allocating VM to the last host again, so adjusting the reserved capacity if it is not less than required");
logger.debug("Reserved CPU: " + reservedCpu + " , Requested CPU: " + cpu);
logger.debug("Reserved RAM: " + toHumanReadableSize(reservedMem) + " , Requested RAM: " + toHumanReadableSize(ram));
}
if (reservedCpu >= cpu && reservedMem >= ram) {
capacityCpu.setReservedCapacity(reservedCpu - cpu);
capacityMem.setReservedCapacity(reservedMem - ram);
capacityCpuCore.setReservedCapacity(reservedCpuCore - cpucore);
}
} else {
/* alloc from free resource */
if (!((reservedCpu + usedCpu + cpu <= totalCpu) && (reservedMem + usedMem + ram <= totalMem))) {
if (logger.isDebugEnabled()) {
logger.debug("Host doesn't seem to have enough free capacity, but increasing the used capacity anyways, " +
"since the VM is already starting on this host ");
}
}
/* alloc from free resource */
if (!((reservedCpu + usedCpu + cpu <= totalCpu) && (reservedMem + usedMem + ram <= totalMem))) {
logger.debug("Host doesn't seem to have enough free capacity, but increasing the used capacity anyways, " +
"since the VM is already starting on this host");
}

logger.debug(String.format("CPU STATS after allocation: for host: %s, " +
"old used: %d, old reserved: %d, actual total: %d, " +
"total with overprovisioning: %d; new used: %d, reserved: %d; " +
"requested cpu: %d, alloc_from_last: %s",
logger.debug("CPU STATS after allocation: for host: {}, " +
"old used: {}, old reserved: {}, actual total: {}, " +
"total with overprovisioning: {}; new used: {}, reserved: {}; " +
"requested cpu: {}",
host, usedCpu, reservedCpu, actualTotalCpu, totalCpu,
capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), cpu, fromLastHost));
capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), cpu);

logger.debug("RAM STATS after allocation: for host: {}, " +
"old used: {}, old reserved: {}, total: {}; new used: {}, reserved: {}; " +
"requested mem: {}, alloc_from_last: {}",
"requested mem: {}",
host, toHumanReadableSize(usedMem), toHumanReadableSize(reservedMem),
toHumanReadableSize(totalMem), toHumanReadableSize(capacityMem.getUsedCapacity()),
toHumanReadableSize(capacityMem.getReservedCapacity()), toHumanReadableSize(ram), fromLastHost);
toHumanReadableSize(capacityMem.getReservedCapacity()), toHumanReadableSize(ram));

long cluster_id = host.getClusterId();
ClusterDetailsVO cluster_detail_cpu = _clusterDetailsDao.findDetail(cluster_id, VmDetailConstants.CPU_OVER_COMMIT_RATIO);
Expand Down Expand Up @@ -959,8 +942,8 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
Host lastHost = _hostDao.findById(vm.getLastHostId());
Host oldHost = _hostDao.findById(oldHostId);
Host newHost = _hostDao.findById(vm.getHostId());
logger.debug(String.format("%s state transited from [%s] to [%s] with event [%s]. VM's original host: %s, new host: %s, host before state transition: %s", vm, oldState,
newState, event, lastHost, newHost, oldHost));
logger.debug("{} state transited from [{}] to [{}] with event [{}]. VM's original host: {}, new host: {}, host before state transition: {}",
vm, oldState, newState, event, lastHost, newHost, oldHost);

if (oldState == State.Starting) {
if (newState != State.Running) {
Expand Down Expand Up @@ -1000,12 +983,10 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
}

if ((newState == State.Starting || newState == State.Migrating || event == Event.AgentReportMigrated) && vm.getHostId() != null) {
boolean fromLastHost = false;
if (vm.getHostId().equals(vm.getLastHostId())) {
logger.debug("VM starting again on the last host it was stopped on");
fromLastHost = true;
if (vm.getLastHostId() != null) {
releaseVmCapacity(vm, true, false, vm.getLastHostId());
}
allocateVmCapacity(vm, fromLastHost);
allocateVmCapacity(vm);
}

if (newState == State.Stopped && event != Event.RestoringFailed && event != Event.RestoringSuccess && vm.getType() == VirtualMachine.Type.User) {
Expand Down
55 changes: 55 additions & 0 deletions server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand All @@ -43,10 +46,15 @@

import com.cloud.dc.ClusterDetailsDao;
import com.cloud.host.Host;
import com.cloud.host.dao.HostDao;
import com.cloud.offering.ServiceOffering;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.utils.Pair;
import com.cloud.utils.fsm.StateMachine2;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachine.Event;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VmDetailConstants;

@RunWith(MockitoJUnitRunner.class)
Expand All @@ -55,6 +63,8 @@ public class CapacityManagerImplTest {
ClusterDetailsDao clusterDetailsDao;
@Mock
ServiceOfferingDao serviceOfferingDao;
@Mock
HostDao hostDao;

@Spy
@InjectMocks
Expand Down Expand Up @@ -160,6 +170,51 @@ public void testCheckIfHostHasNoCpuCapabilityButHasCapacity() {
eq(false), eq(cpuOvercommit), eq(memoryOvercommit), eq(false));
}

@Test
public void testPostStateTransitionReleasesStaleReservationWhenStartingOnDifferentHost() {
final Long oldHostId = 1L;
final Long newHostId = 2L;

VirtualMachine vm = mock(VirtualMachine.class);
when(vm.getHostId()).thenReturn(newHostId);
when(vm.getLastHostId()).thenReturn(oldHostId);

doNothing().when(capacityManager).allocateVmCapacity(any(VirtualMachine.class));
doReturn(true).when(capacityManager).releaseVmCapacity(
any(VirtualMachine.class), anyBoolean(), anyBoolean(), anyLong());

StateMachine2.Transition<State, Event> transition = new StateMachine2.Transition<>(
State.Stopped, Event.StartRequested, State.Starting, Collections.emptyList());
Pair<Long, Long> opaque = new Pair<>(null, 0L);

capacityManager.postStateTransitionEvent(transition, vm, true, opaque);

verify(capacityManager).releaseVmCapacity(vm, true, false, oldHostId);
verify(capacityManager).allocateVmCapacity(vm);
}

@Test
public void testPostStateTransitionReleasesReservationWhenStartingOnSameHost() {
final Long hostId = 1L;

VirtualMachine vm = mock(VirtualMachine.class);
when(vm.getHostId()).thenReturn(hostId);
when(vm.getLastHostId()).thenReturn(hostId);

doNothing().when(capacityManager).allocateVmCapacity(any(VirtualMachine.class));
doReturn(true).when(capacityManager).releaseVmCapacity(
any(VirtualMachine.class), anyBoolean(), anyBoolean(), anyLong());

StateMachine2.Transition<State, Event> transition = new StateMachine2.Transition<>(
State.Stopped, Event.StartRequested, State.Starting, Collections.emptyList());
Pair<Long, Long> opaque = new Pair<>(null, 0L);

capacityManager.postStateTransitionEvent(transition, vm, true, opaque);

verify(capacityManager).releaseVmCapacity(vm, true, false, hostId);
verify(capacityManager).allocateVmCapacity(vm);
}

@Test
public void testCheckIfHostHasCpuCapabilityButNoCapacity() {
Float cpuOvercommit = 2.0f;
Expand Down