Skip to content

Commit b984184

Browse files
DaanHooglandDaanHoogland
committed
Merge release branch 4.13 to master
* 4.13: Snapshot deletion issues (#3969) server: Cannot list affinity group if there are hosts dedicated… (#4025) server: Search zone-wide storage pool when allocation algothrim is firstfitleastconsumed (#4002)
2 parents 3d4b9af + f18fe5e commit b984184

11 files changed

Lines changed: 225 additions & 91 deletions

File tree

‎engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,5 @@ List<SummedCapacity> findCapacityBy(Integer capacityType, Long zoneId,
5656

5757
float findClusterConsumption(Long clusterId, short capacityType, long computeRequested);
5858

59-
List<Long> orderHostsByFreeCapacity(Long clusterId, short capacityType);
59+
List<Long> orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityType);
6060
}

‎engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -903,20 +903,28 @@ public Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long
903903
}
904904

905905
@Override
906-
public List<Long> orderHostsByFreeCapacity(Long clusterId, short capacityTypeForOrdering){
906+
public List<Long> orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityTypeForOrdering){
907907
TransactionLegacy txn = TransactionLegacy.currentTxn();
908908
PreparedStatement pstmt = null;
909909
List<Long> result = new ArrayList<Long>();
910910
StringBuilder sql = new StringBuilder(ORDER_HOSTS_BY_FREE_CAPACITY_PART1);
911-
if(clusterId != null) {
912-
sql.append("AND cluster_id = ?");
913-
}
914-
sql.append(ORDER_HOSTS_BY_FREE_CAPACITY_PART2);
911+
if (zoneId != null) {
912+
sql.append(" AND data_center_id = ?");
913+
}
914+
if (clusterId != null) {
915+
sql.append(" AND cluster_id = ?");
916+
}
917+
sql.append(ORDER_HOSTS_BY_FREE_CAPACITY_PART2);
915918
try {
916919
pstmt = txn.prepareAutoCloseStatement(sql.toString());
917920
pstmt.setShort(1, capacityTypeForOrdering);
918-
if(clusterId != null) {
919-
pstmt.setLong(2, clusterId);
921+
int index = 2;
922+
if (zoneId != null) {
923+
pstmt.setLong(index, zoneId);
924+
index ++;
925+
}
926+
if (clusterId != null) {
927+
pstmt.setLong(index, clusterId);
920928
}
921929

922930
ResultSet rs = pstmt.executeQuery();

‎engine/storage/integration-test/src/test/resources/fakeDriverTestContext.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
<bean id="dataStoreProviderManagerImpl" class="org.apache.cloudstack.storage.datastore.provider.DataStoreProviderManagerImpl" />
5050
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
5151
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
52-
<bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
52+
<bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
5353
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
5454
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
5555
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />

‎engine/storage/integration-test/src/test/resources/storageContext.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
<bean id="ancientDataMotionStrategy" class="org.apache.cloudstack.storage.motion.AncientDataMotionStrategy" />
5151
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
5252
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
53-
<bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
53+
<bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
5454
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
5555
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
5656
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java renamed to engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java

Lines changed: 124 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import javax.inject.Inject;
2222

2323
import org.apache.log4j.Logger;
24-
import org.springframework.stereotype.Component;
2524
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2625
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
2726
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
@@ -34,7 +33,6 @@
3433
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
3534
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
3635
import org.apache.cloudstack.framework.jobs.AsyncJob;
37-
import org.apache.cloudstack.framework.jobs.dao.SyncQueueItemDao;
3836
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
3937
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
4038
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
@@ -71,9 +69,8 @@
7169
import com.cloud.utils.exception.CloudRuntimeException;
7270
import com.cloud.utils.fsm.NoTransitionException;
7371

74-
@Component
75-
public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
76-
private static final Logger s_logger = Logger.getLogger(XenserverSnapshotStrategy.class);
72+
public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
73+
private static final Logger s_logger = Logger.getLogger(DefaultSnapshotStrategy.class);
7774

7875
@Inject
7976
SnapshotService snapshotSvr;
@@ -90,12 +87,8 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
9087
@Inject
9188
SnapshotDataFactory snapshotDataFactory;
9289
@Inject
93-
private SnapshotDao _snapshotDao;
94-
@Inject
9590
private SnapshotDetailsDao _snapshotDetailsDao;
9691
@Inject
97-
private SyncQueueItemDao _syncQueueItemDao;
98-
@Inject
9992
VolumeDetailsDao _volumeDetailsDaoImpl;
10093

10194
@Override
@@ -264,68 +257,147 @@ public boolean deleteSnapshot(Long snapshotId) {
264257
return true;
265258
}
266259

267-
if (!Snapshot.State.BackedUp.equals(snapshotVO.getState()) && !Snapshot.State.Error.equals(snapshotVO.getState()) &&
260+
if (!Snapshot.State.BackedUp.equals(snapshotVO.getState()) &&
268261
!Snapshot.State.Destroying.equals(snapshotVO.getState())) {
269262
throw new InvalidParameterValueException("Can't delete snapshotshot " + snapshotId + " due to it is in " + snapshotVO.getState() + " Status");
270263
}
271264

272-
// first mark the snapshot as destroyed, so that ui can't see it, but we
273-
// may not destroy the snapshot on the storage, as other snapshots may
274-
// depend on it.
275-
SnapshotInfo snapshotOnImage = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Image);
276-
if (snapshotOnImage == null) {
277-
s_logger.debug("Can't find snapshot on backup storage, delete it in db");
278-
snapshotDao.remove(snapshotId);
279-
return true;
280-
}
265+
Boolean deletedOnSecondary = deleteOnSecondaryIfNeeded(snapshotId);
266+
boolean deletedOnPrimary = deleteOnPrimaryIfNeeded(snapshotId);
281267

282-
SnapshotObject obj = (SnapshotObject)snapshotOnImage;
283-
try {
284-
obj.processEvent(Snapshot.Event.DestroyRequested);
285-
List<VolumeDetailVO> volumesFromSnapshot;
286-
volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
268+
if (deletedOnPrimary) {
269+
s_logger.debug(String.format("Successfully deleted snapshot (id: %d) on primary storage.", snapshotId));
270+
} else {
271+
s_logger.debug(String.format("The snapshot (id: %d) could not be found/deleted on primary storage.", snapshotId));
272+
}
273+
if (null != deletedOnSecondary && deletedOnSecondary) {
274+
s_logger.debug(String.format("Successfully deleted snapshot (id: %d) on secondary storage.", snapshotId));
275+
}
276+
return (deletedOnSecondary != null) && deletedOnSecondary || deletedOnPrimary;
277+
}
287278

288-
if (volumesFromSnapshot.size() > 0) {
279+
private boolean deleteOnPrimaryIfNeeded(Long snapshotId) {
280+
SnapshotVO snapshotVO;
281+
boolean deletedOnPrimary = false;
282+
snapshotVO = snapshotDao.findById(snapshotId);
283+
SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
284+
if (snapshotVO != null && snapshotVO.getState() == Snapshot.State.Destroyed) {
285+
deletedOnPrimary = deleteSnapshotOnPrimary(snapshotId, snapshotOnPrimaryInfo);
286+
} else {
287+
// Here we handle snapshots which are to be deleted but are not marked as destroyed yet.
288+
// This may occur for instance when they are created only on primary because
289+
// snapshot.backup.to.secondary was set to false.
290+
if (snapshotOnPrimaryInfo == null) {
291+
if (s_logger.isDebugEnabled()) {
292+
s_logger.debug(String.format("Snapshot (id: %d) not found on primary storage, skipping snapshot deletion on primary and marking it destroyed", snapshotId));
293+
}
294+
snapshotVO.setState(Snapshot.State.Destroyed);
295+
snapshotDao.update(snapshotId, snapshotVO);
296+
deletedOnPrimary = true;
297+
} else {
298+
SnapshotObject obj = (SnapshotObject) snapshotOnPrimaryInfo;
289299
try {
290-
obj.processEvent(Snapshot.Event.OperationFailed);
291-
} catch (NoTransitionException e1) {
292-
s_logger.debug("Failed to change snapshot state: " + e1.toString());
300+
obj.processEvent(Snapshot.Event.DestroyRequested);
301+
deletedOnPrimary = deleteSnapshotOnPrimary(snapshotId, snapshotOnPrimaryInfo);
302+
if (!deletedOnPrimary) {
303+
obj.processEvent(Snapshot.Event.OperationFailed);
304+
} else {
305+
obj.processEvent(Snapshot.Event.OperationSucceeded);
306+
}
307+
} catch (NoTransitionException e) {
308+
s_logger.debug("Failed to set the state to destroying: ", e);
309+
deletedOnPrimary = false;
293310
}
294-
throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
295311
}
296-
} catch (NoTransitionException e) {
297-
s_logger.debug("Failed to set the state to destroying: ", e);
298-
return false;
299312
}
313+
return deletedOnPrimary;
314+
}
300315

301-
try {
302-
boolean result = deleteSnapshotChain(snapshotOnImage);
303-
obj.processEvent(Snapshot.Event.OperationSucceeded);
304-
if (result) {
305-
//snapshot is deleted on backup storage, need to delete it on primary storage
306-
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
307-
if (snapshotOnPrimary != null) {
308-
SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
309-
long volumeId = snapshotOnPrimary.getVolumeId();
310-
VolumeVO volumeVO = volumeDao.findById(volumeId);
311-
if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
312-
snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
313-
}
314-
snapshotOnPrimary.setState(State.Destroyed);
315-
snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
316+
private Boolean deleteOnSecondaryIfNeeded(Long snapshotId) {
317+
Boolean deletedOnSecondary = null;
318+
SnapshotInfo snapshotOnImage = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Image);
319+
if (snapshotOnImage == null) {
320+
s_logger.debug(String.format("Can't find snapshot [snapshot id: %d] on secondary storage", snapshotId));
321+
} else {
322+
SnapshotObject obj = (SnapshotObject)snapshotOnImage;
323+
try {
324+
deletedOnSecondary = deleteSnapshotOnSecondaryStorage(snapshotId, snapshotOnImage, obj);
325+
if (!deletedOnSecondary) {
326+
s_logger.debug(
327+
String.format("Failed to find/delete snapshot (id: %d) on secondary storage. Still necessary to check and delete snapshot on primary storage.",
328+
snapshotId));
329+
} else {
330+
s_logger.debug(String.format("Snapshot (id: %d) has been deleted on secondary storage.", snapshotId));
316331
}
332+
} catch (NoTransitionException e) {
333+
s_logger.debug("Failed to set the state to destroying: ", e);
334+
// deletedOnSecondary remain null
317335
}
318-
} catch (Exception e) {
319-
s_logger.debug("Failed to delete snapshot: ", e);
336+
}
337+
return deletedOnSecondary;
338+
}
339+
340+
/**
341+
* Deletes the snapshot on secondary storage.
342+
* It can return false when the snapshot was stored on primary storage and not backed up on secondary; therefore, the snapshot should also be deleted on primary storage even when this method returns false.
343+
*/
344+
private boolean deleteSnapshotOnSecondaryStorage(Long snapshotId, SnapshotInfo snapshotOnImage, SnapshotObject obj) throws NoTransitionException {
345+
obj.processEvent(Snapshot.Event.DestroyRequested);
346+
List<VolumeDetailVO> volumesFromSnapshot;
347+
volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
348+
349+
if (volumesFromSnapshot.size() > 0) {
320350
try {
321351
obj.processEvent(Snapshot.Event.OperationFailed);
322352
} catch (NoTransitionException e1) {
323-
s_logger.debug("Failed to change snapshot state: " + e.toString());
353+
s_logger.debug("Failed to change snapshot state: " + e1.toString());
354+
}
355+
throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
356+
}
357+
358+
boolean result = deleteSnapshotChain(snapshotOnImage);
359+
obj.processEvent(Snapshot.Event.OperationSucceeded);
360+
return result;
361+
}
362+
363+
/**
364+
* Deletes the snapshot on primary storage. It returns true when the snapshot was not found on primary storage; </br>
365+
* In case of failure while deleting the snapshot, it will throw one of the following exceptions: CloudRuntimeException, InterruptedException, or ExecutionException. </br>
366+
*/
367+
private boolean deleteSnapshotOnPrimary(Long snapshotId, SnapshotInfo snapshotOnPrimaryInfo) {
368+
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
369+
if (isSnapshotOnPrimaryStorage(snapshotId)) {
370+
if (s_logger.isDebugEnabled()) {
371+
s_logger.debug(String.format("Snapshot reference is found on primary storage for snapshot id: %d, performing snapshot deletion on primary", snapshotId));
372+
}
373+
if (snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo)) {
374+
snapshotOnPrimary.setState(State.Destroyed);
375+
snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
376+
if (s_logger.isDebugEnabled()) {
377+
s_logger.debug(String.format("Successfully deleted snapshot id: %d on primary storage", snapshotId));
378+
}
379+
return true;
324380
}
325-
return false;
381+
} else {
382+
if (s_logger.isDebugEnabled()) {
383+
s_logger.debug(String.format("Snapshot reference is not found on primary storage for snapshot id: %d, skipping snapshot deletion on primary", snapshotId));
384+
}
385+
return true;
326386
}
387+
return false;
388+
}
327389

328-
return true;
390+
/**
391+
* Returns true if the snapshot volume is on primary storage.
392+
*/
393+
private boolean isSnapshotOnPrimaryStorage(long snapshotId) {
394+
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
395+
if (snapshotOnPrimary != null) {
396+
long volumeId = snapshotOnPrimary.getVolumeId();
397+
VolumeVO volumeVO = volumeDao.findById(volumeId);
398+
return volumeVO != null && volumeVO.getRemoved() == null;
399+
}
400+
return false;
329401
}
330402

331403
@Override

‎engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
http://www.springframework.org/schema/context/spring-context.xsd"
2828
>
2929

30-
<bean id="xenserverSnapshotStrategy"
31-
class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
30+
<bean id="defaultSnapshotStrategy"
31+
class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
3232

3333
<bean id="storageSystemSnapshotStrategy"
3434
class="org.apache.cloudstack.storage.snapshot.StorageSystemSnapshotStrategy" />

‎engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile
9494

9595
protected List<StoragePool> reorderPoolsByCapacity(DeploymentPlan plan,
9696
List<StoragePool> pools) {
97+
Long zoneId = plan.getDataCenterId();
9798
Long clusterId = plan.getClusterId();
9899
short capacityType;
99100
if(pools != null && pools.size() != 0){
@@ -102,7 +103,7 @@ protected List<StoragePool> reorderPoolsByCapacity(DeploymentPlan plan,
102103
return null;
103104
}
104105

105-
List<Long> poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(clusterId, capacityType);
106+
List<Long> poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType);
106107
if (s_logger.isDebugEnabled()) {
107108
s_logger.debug("List of pools in descending order of free capacity: "+ poolIdsByCapacity);
108109
}

‎engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
3030
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
3131

32+
import com.cloud.capacity.Capacity;
33+
import com.cloud.capacity.dao.CapacityDao;
3234
import com.cloud.deploy.DeploymentPlan;
3335
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
3436
import com.cloud.hypervisor.Hypervisor.HypervisorType;
@@ -43,6 +45,8 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator {
4345
private static final Logger LOGGER = Logger.getLogger(ZoneWideStoragePoolAllocator.class);
4446
@Inject
4547
private DataStoreManager dataStoreMgr;
48+
@Inject
49+
private CapacityDao capacityDao;
4650

4751

4852
@Override
@@ -110,6 +114,40 @@ private boolean canAddStoragePoolToAvoidSet(StoragePoolVO storagePoolVO) {
110114
return !ScopeType.ZONE.equals(storagePoolVO.getScope()) || !storagePoolVO.isManaged();
111115
}
112116

117+
118+
@Override
119+
protected List<StoragePool> reorderPoolsByCapacity(DeploymentPlan plan,
120+
List<StoragePool> pools) {
121+
Long zoneId = plan.getDataCenterId();
122+
short capacityType;
123+
if(pools != null && pools.size() != 0){
124+
capacityType = pools.get(0).getPoolType().isShared() ? Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED : Capacity.CAPACITY_TYPE_LOCAL_STORAGE;
125+
} else{
126+
return null;
127+
}
128+
129+
List<Long> poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, null, capacityType);
130+
if (LOGGER.isDebugEnabled()) {
131+
LOGGER.debug("List of zone-wide storage pools in descending order of free capacity: "+ poolIdsByCapacity);
132+
}
133+
134+
//now filter the given list of Pools by this ordered list
135+
Map<Long, StoragePool> poolMap = new HashMap<>();
136+
for (StoragePool pool : pools) {
137+
poolMap.put(pool.getId(), pool);
138+
}
139+
List<Long> matchingPoolIds = new ArrayList<>(poolMap.keySet());
140+
141+
poolIdsByCapacity.retainAll(matchingPoolIds);
142+
143+
List<StoragePool> reorderedPools = new ArrayList<>();
144+
for(Long id: poolIdsByCapacity){
145+
reorderedPools.add(poolMap.get(id));
146+
}
147+
148+
return reorderedPools;
149+
}
150+
113151
@Override
114152
protected List<StoragePool> reorderPoolsByNumberOfVolumes(DeploymentPlan plan, List<StoragePool> pools, Account account) {
115153
if (account == null) {

0 commit comments

Comments
 (0)