apache/cloudstack
1
0
Fork 0
mirror of https://github.com/apache/cloudstack.git synced 2025-10-26 08:42:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

server: improve storage GC to skip expunging possible duplicate volumes (#7313)

Browse Source 
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This commit is contained in:
Abhishek Kumar 2023-06-05 13:33:24 +05:30 committed by GitHub
parent b2e9993b0a
commit 2d6a069812
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 104 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
server/src/main/java/com/cloud/storage/StorageManagerImpl.java
View File

@ -19,6 +19,7 @@ package com.cloud.storage;
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
@ -37,6 +38,7 @@ import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
@ -45,8 +47,6 @@ import java.util.stream.Collectors;
import javax.inject.Inject;
import com.google.common.collect.Sets;
import com.cloud.vm.UserVmManager;
import org.apache.cloudstack.annotation.AnnotationService;
import org.apache.cloudstack.annotation.dao.AnnotationDao;
import org.apache.cloudstack.api.ApiConstants;
@ -229,11 +229,11 @@ import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.db.TransactionStatus;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.UserVmManager;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.dao.VMInstanceDao;
import java.math.BigInteger;
import java.util.UUID;
import com.google.common.collect.Sets;
@Component
public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener, Configurable {
@ -1336,7 +1336,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
continue;
}
}
if (isVolumeSuspectedDestroyDuplicateOfVmVolume(vol)) {
s_logger.warn(String.format("Skipping cleaning up %s as it could be a duplicate for another volume on same pool", vol));
continue;
}
try {
// If this fails, just log a warning. It's ideal if we clean up the host-side clustered file
// system, but not necessary.
@ -1468,6 +1471,31 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
}
}
protected boolean isVolumeSuspectedDestroyDuplicateOfVmVolume(VolumeVO gcVolume) {
if (gcVolume.getPath() == null) {
return false;
}
if (gcVolume.getPoolId() == null) {
return false;
}
Long vmId = gcVolume.getInstanceId();
if (vmId == null) {
return false;
}
VMInstanceVO vm = _vmInstanceDao.findById(vmId);
if (vm == null) {
return false;
}
List<VolumeVO> vmUsableVolumes = _volumeDao.findUsableVolumesForInstance(vmId);
for (VolumeVO vol : vmUsableVolumes) {
if (gcVolume.getPoolId().equals(vol.getPoolId()) && gcVolume.getPath().equals(vol.getPath())) {
s_logger.debug(String.format("%s meant for garbage collection could a possible duplicate for %s", gcVolume, vol));
return true;
}
}
return false;
}
/**
* This method only applies for managed storage.
*

72
server/src/test/java/com/cloud/storage/StorageManagerImplTest.java
View File

@ -16,20 +16,35 @@
// under the License.
package com.cloud.storage;
import java.util.ArrayList;
import java.util.List;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.junit.MockitoJUnitRunner;
import com.cloud.agent.api.StoragePoolInfo;
import com.cloud.host.Host;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.dao.VMInstanceDao;
@RunWith(MockitoJUnitRunner.class)
public class StorageManagerImplTest {
@Mock
VolumeDao _volumeDao;
@Mock
VMInstanceDao vmInstanceDao;
@Spy
@InjectMocks
private StorageManagerImpl storageManagerImpl;
@Test
@ -58,4 +73,59 @@ public class StorageManagerImplTest {
String localStoragePoolName = storageManagerImpl.createLocalStoragePoolName(hostMock, storagePoolInfoMock);
Assert.assertEquals(expectedLocalStorageName, localStoragePoolName);
}
private VolumeVO mockVolumeForIsVolumeSuspectedDestroyDuplicateTest() {
VolumeVO volumeVO = new VolumeVO("data", 1L, 1L, 1L, 1L, 1L, "data", "data", Storage.ProvisioningType.THIN, 1, null, null, "data", Volume.Type.DATADISK);
volumeVO.setPoolId(1L);
return volumeVO;
}
@Test
public void testIsVolumeSuspectedDestroyDuplicateNoPool() {
VolumeVO volume = mockVolumeForIsVolumeSuspectedDestroyDuplicateTest();
volume.setPoolId(null);
Assert.assertFalse(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume));
}
@Test
public void testIsVolumeSuspectedDestroyDuplicateNoPath() {
VolumeVO volume = mockVolumeForIsVolumeSuspectedDestroyDuplicateTest();
Assert.assertFalse(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume));
}
@Test
public void testIsVolumeSuspectedDestroyDuplicateNoVmId() {
VolumeVO volume = mockVolumeForIsVolumeSuspectedDestroyDuplicateTest();
volume.setInstanceId(null);
Assert.assertFalse(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume));
}
@Test
public void testIsVolumeSuspectedDestroyDuplicateNoVm() {
VolumeVO volume = mockVolumeForIsVolumeSuspectedDestroyDuplicateTest();
Assert.assertFalse(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume));
}
@Test
public void testIsVolumeSuspectedDestroyDuplicateNoVmVolumes() {
VolumeVO volume = mockVolumeForIsVolumeSuspectedDestroyDuplicateTest();
Mockito.when(vmInstanceDao.findById(1L)).thenReturn(Mockito.mock(VMInstanceVO.class));
Mockito.when(_volumeDao.findUsableVolumesForInstance(1L)).thenReturn(new ArrayList<>());
Assert.assertFalse(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume));
}
@Test
public void testIsVolumeSuspectedDestroyDuplicateTrue() {
Long poolId = 1L;
String path = "data";
VolumeVO volume = mockVolumeForIsVolumeSuspectedDestroyDuplicateTest();
volume.setPoolId(poolId);
Mockito.when(vmInstanceDao.findById(1L)).thenReturn(Mockito.mock(VMInstanceVO.class));
VolumeVO volumeVO = Mockito.mock(VolumeVO.class);
Mockito.when(volumeVO.getPoolId()).thenReturn(poolId);
Mockito.when(volumeVO.getPath()).thenReturn(path);
Mockito.when(_volumeDao.findUsableVolumesForInstance(1L)).thenReturn(List.of(volumeVO, Mockito.mock(VolumeVO.class)));
Assert.assertTrue(storageManagerImpl.isVolumeSuspectedDestroyDuplicateOfVmVolume(volume));
}
}