Skip to content

Commit 7efe8fb

Browse files
Cleanup snapshots in datastores for Error-ed snapshots, and some code improvements
1 parent 56a39e6 commit 7efe8fb

File tree

15 files changed

+125
-83
lines changed

15 files changed

+125
-83
lines changed

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public interface SnapshotDataFactory {
3030

3131
SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole role);
3232

33+
SnapshotInfo getSnapshotIncludingRemoved(long snapshotId, long storeId, DataStoreRole role);
34+
3335
SnapshotInfo getSnapshotWithRoleAndZone(long snapshotId, DataStoreRole role, long zoneId);
3436

3537
SnapshotInfo getSnapshotOnPrimaryStore(long snapshotId);

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap
4747

4848
List<SnapshotVO> listAllByStatus(Snapshot.State... status);
4949

50+
List<SnapshotVO> listAllByStatusIncludingRemoved(Snapshot.State... status);
51+
5052
void updateVolumeIds(long oldVolId, long newVolId);
5153

5254
List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status);

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,13 @@ public List<SnapshotVO> listAllByStatus(Snapshot.State... status) {
252252
return listBy(sc, null);
253253
}
254254

255+
@Override
256+
public List<SnapshotVO> listAllByStatusIncludingRemoved(Snapshot.State... status) {
257+
SearchCriteria<SnapshotVO> sc = StatusSearch.create();
258+
sc.setParameters("status", (Object[])status);
259+
return listIncludingRemovedBy(sc, null);
260+
}
261+
255262
@Override
256263
public List<SnapshotVO> listByIds(Object... ids) {
257264
SearchCriteria<SnapshotVO> sc = snapshotIdsSearch.create();

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
5656

5757
List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId);
5858

59+
List<SnapshotDataStoreVO> findBySnapshotIdWithNonDestroyedState(long snapshotId);
60+
5961
void duplicateCacheRecordsOnRegionStore(long storeId);
6062

6163
// delete the snapshot entry on primary data store to make sure that next snapshot will be full snapshot

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,13 @@ public List<SnapshotDataStoreVO> findByVolume(long snapshotId, long volumeId, Da
340340

341341
@Override
342342
public List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId) {
343+
SearchCriteria<SnapshotDataStoreVO> sc = searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq.create();
344+
sc.setParameters(SNAPSHOT_ID, snapshotId);
345+
return listBy(sc);
346+
}
347+
348+
@Override
349+
public List<SnapshotDataStoreVO> findBySnapshotIdWithNonDestroyedState(long snapshotId) {
343350
SearchCriteria<SnapshotDataStoreVO> sc = idStateNeqSearch.create();
344351
sc.setParameters(SNAPSHOT_ID, snapshotId);
345352
sc.setParameters(STATE, State.Destroyed);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
270270
}
271271

272272
if (Snapshot.State.Error.equals(snapshotVO.getState())) {
273-
List<SnapshotDataStoreVO> storeRefs = snapshotStoreDao.findBySnapshotId(snapshotId);
273+
List<SnapshotDataStoreVO> storeRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
274274
List<Long> deletedRefs = new ArrayList<>();
275275
for (SnapshotDataStoreVO ref : storeRefs) {
276276
boolean refZoneIdMatch = false;
@@ -351,7 +351,7 @@ protected boolean deleteSnapshotInfos(SnapshotVO snapshotVo, Long zoneId) {
351351
protected Boolean deleteSnapshotInfo(SnapshotInfo snapshotInfo, SnapshotVO snapshotVo) {
352352
DataStore dataStore = snapshotInfo.getDataStore();
353353
String storageToString = String.format("%s {uuid: \"%s\", name: \"%s\"}", dataStore.getRole().name(), dataStore.getUuid(), dataStore.getName());
354-
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotVo.getId());
354+
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotVo.getId());
355355
boolean isLastSnapshotRef = CollectionUtils.isEmpty(snapshotStoreRefs) || snapshotStoreRefs.size() == 1;
356356
try {
357357
SnapshotObject snapshotObject = castSnapshotInfoToSnapshotObject(snapshotInfo);

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public List<SnapshotInfo> getSnapshots(long snapshotId, Long zoneId) {
9494
if (snapshot == null) { //snapshot may have been removed;
9595
return new ArrayList<>();
9696
}
97-
List<SnapshotDataStoreVO> allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotId(snapshotId);
97+
List<SnapshotDataStoreVO> allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
9898
if (CollectionUtils.isEmpty(allSnapshotsAndDataStore)) {
9999
return new ArrayList<>();
100100
}
@@ -118,7 +118,23 @@ public SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole rol
118118
if (snapshot == null) {
119119
return null;
120120
}
121-
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByStoreSnapshot(role, storeId, snapshotId);
121+
return getSnapshotOnStore(snapshot, storeId, role);
122+
}
123+
124+
@Override
125+
public SnapshotInfo getSnapshotIncludingRemoved(long snapshotId, long storeId, DataStoreRole role) {
126+
SnapshotVO snapshot = snapshotDao.findByIdIncludingRemoved(snapshotId);
127+
if (snapshot == null) {
128+
return null;
129+
}
130+
return getSnapshotOnStore(snapshot, storeId, role);
131+
}
132+
133+
private SnapshotInfo getSnapshotOnStore(SnapshotVO snapshot, long storeId, DataStoreRole role) {
134+
if (snapshot == null) {
135+
return null;
136+
}
137+
SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByStoreSnapshot(role, storeId, snapshot.getId());
122138
if (snapshotStore == null) {
123139
return null;
124140
}
@@ -207,7 +223,7 @@ public List<SnapshotInfo> listSnapshotOnCache(long snapshotId) {
207223

208224
@Override
209225
public void updateOperationFailed(long snapshotId) throws NoTransitionException {
210-
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotId);
226+
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
211227
for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) {
212228
SnapshotInfo snapshotInfo = getSnapshot(snapshotStoreRef.getSnapshotId(), snapshotStoreRef.getDataStoreId(), snapshotStoreRef.getRole());
213229
if (snapshotInfo != null) {

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -382,16 +382,14 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) {
382382
if (res.isFailed()) {
383383
throw new CloudRuntimeException(res.getResult());
384384
}
385-
SnapshotInfo destSnapshot = res.getSnapshot();
386-
return destSnapshot;
385+
return res.getSnapshot();
387386
} catch (InterruptedException e) {
388387
logger.debug("failed copy snapshot", e);
389388
throw new CloudRuntimeException("Failed to copy snapshot", e);
390389
} catch (ExecutionException e) {
391390
logger.debug("Failed to copy snapshot", e);
392391
throw new CloudRuntimeException("Failed to copy snapshot", e);
393392
}
394-
395393
}
396394

397395
protected Void copySnapshotAsyncCallback(AsyncCallbackDispatcher<SnapshotServiceImpl, CopyCommandResult> callback, CopySnapshotContext<CommandResult> context) {
@@ -479,7 +477,6 @@ protected Void prepareCopySnapshotZoneAsyncCallback(AsyncCallbackDispatcher<Snap
479477
}
480478

481479
protected Void deleteSnapshotCallback(AsyncCallbackDispatcher<SnapshotServiceImpl, CommandResult> callback, DeleteSnapshotContext<CommandResult> context) {
482-
483480
CommandResult result = callback.getResult();
484481
AsyncCallFuture<SnapshotResult> future = context.future;
485482
SnapshotInfo snapshot = context.snapshot;
@@ -607,7 +604,7 @@ public void cleanupVolumeDuringSnapshotFailure(Long volumeId, Long snapshotId) {
607604

608605
if (snapshot != null) {
609606
if (snapshot.getState() != Snapshot.State.BackedUp) {
610-
List<SnapshotDataStoreVO> snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotId(snapshotId);
607+
List<SnapshotDataStoreVO> snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
611608
for (SnapshotDataStoreVO snapshotDataStoreVO : snapshotDataStoreVOs) {
612609
logger.debug("Remove snapshot {}, status {} on snapshot_store_ref table with id: {}", snapshot, snapshotDataStoreVO.getState(), snapshotDataStoreVO.getId());
613610

@@ -712,7 +709,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
712709
SnapshotObject srcSnapshot = (SnapshotObject)snapshot;
713710
srcSnapshot.processEvent(Event.DestroyRequested);
714711
srcSnapshot.processEvent(Event.OperationSuccessed);
715-
716712
srcSnapshot.processEvent(Snapshot.Event.OperationFailed);
717713

718714
_snapshotDetailsDao.removeDetail(srcSnapshot.getId(), AsyncJob.Constants.MS_ID);
@@ -723,7 +719,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
723719
}
724720
}
725721
});
726-
727722
}
728723

729724
@Override

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,6 @@ public void postSnapshotCreation(SnapshotInfo snapshot) {
540540
logger.warn("Failed to clean up snapshot '" + snapshot.getId() + "' on primary storage: " + e.getMessage());
541541
}
542542
}
543-
544543
}
545544

546545
private VMSnapshot takeHypervisorSnapshot(VolumeInfo volumeInfo) {

engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ public ObjectInDataStoreManagerImpl() {
101101
stateMachines.addTransition(State.Destroying, Event.DestroyRequested, State.Destroying);
102102
stateMachines.addTransition(State.Destroying, Event.OperationSuccessed, State.Destroyed);
103103
stateMachines.addTransition(State.Destroying, Event.OperationFailed, State.Destroying);
104+
stateMachines.addTransition(State.Destroyed, Event.DestroyRequested, State.Destroyed);
105+
stateMachines.addTransition(State.Destroyed, Event.OperationSuccessed, State.Destroyed);
106+
stateMachines.addTransition(State.Destroyed, Event.OperationFailed, State.Destroyed);
104107
stateMachines.addTransition(State.Failed, Event.DestroyRequested, State.Destroying);
105108
// TODO: further investigate why an extra event is sent when it is
106109
// already Ready for DownloadListener

0 commit comments

Comments
 (0)