Skip to content

Commit 27133fb

Browse files
minchen07minchen07
committed
Simplify clean up snapshots logic in secondary storage and consolidate
to use one agent command DeleteSnapshotBackupCommand for snapshot deletion task by removing CleanupSnapshotBackupCommand.
1 parent 0ed441c commit 27133fb

5 files changed

Lines changed: 123 additions & 224 deletions

File tree

‎core/src/com/cloud/agent/api/CleanupSnapshotBackupCommand.java‎

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎engine/api/src/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
3535

3636
public SnapshotDataStoreVO findByStoreSnapshot(DataStoreRole role, long storeId, long snapshotId);
3737

38-
public SnapshotDataStoreVO findByStoreSnapshot(long storeId, long snapshotId, boolean lock);
39-
4038
public SnapshotDataStoreVO findBySnapshot(long snapshotId, DataStoreRole role);
39+
40+
public List<SnapshotDataStoreVO> listDestroyed(long storeId);
4141
}

‎engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import javax.naming.ConfigurationException;
2323

2424
import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
25+
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
2526
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
2627
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State;
2728
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
@@ -44,6 +45,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
4445
private static final Logger s_logger = Logger.getLogger(SnapshotDataStoreDaoImpl.class);
4546
private SearchBuilder<SnapshotDataStoreVO> updateStateSearch;
4647
private SearchBuilder<SnapshotDataStoreVO> storeSearch;
48+
private SearchBuilder<SnapshotDataStoreVO> destroyedSearch;
4749
private SearchBuilder<SnapshotDataStoreVO> snapshotSearch;
4850
private SearchBuilder<SnapshotDataStoreVO> storeSnapshotSearch;
4951

@@ -54,9 +56,14 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
5456
storeSearch = createSearchBuilder();
5557
storeSearch.and("store_id", storeSearch.entity().getDataStoreId(), SearchCriteria.Op.EQ);
5658
storeSearch.and("store_role", storeSearch.entity().getRole(), SearchCriteria.Op.EQ);
57-
5859
storeSearch.done();
5960

61+
destroyedSearch = createSearchBuilder();
62+
destroyedSearch.and("store_id", destroyedSearch.entity().getDataStoreId(), SearchCriteria.Op.EQ);
63+
destroyedSearch.and("store_role", destroyedSearch.entity().getRole(), SearchCriteria.Op.EQ);
64+
destroyedSearch.and("state", destroyedSearch.entity().getState(), SearchCriteria.Op.EQ);
65+
destroyedSearch.done();
66+
6067
updateStateSearch = this.createSearchBuilder();
6168
updateStateSearch.and("id", updateStateSearch.entity().getId(), Op.EQ);
6269
updateStateSearch.and("state", updateStateSearch.entity().getState(), Op.EQ);
@@ -150,9 +157,14 @@ public SnapshotDataStoreVO findBySnapshot(long snapshotId, DataStoreRole role) {
150157
return findOneBy(sc);
151158
}
152159

160+
161+
153162
@Override
154-
public SnapshotDataStoreVO findByStoreSnapshot(long storeId, long snapshotId, boolean lock) {
155-
// TODO Auto-generated method stub
156-
return null;
163+
public List<SnapshotDataStoreVO> listDestroyed(long id) {
164+
SearchCriteria<SnapshotDataStoreVO> sc = destroyedSearch.create();
165+
sc.setParameters("store_id", id);
166+
sc.setParameters("store_role", DataStoreRole.Image);
167+
sc.setParameters("state", ObjectInDataStoreStateMachine.State.Destroyed);
168+
return listBy(sc);
157169
}
158170
}

‎server/src/com/cloud/storage/StorageManagerImpl.java‎

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
6060
import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
6161
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
62+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
6263
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
6364
import org.apache.cloudstack.engine.subsystem.api.storage.ImageStoreProvider;
6465
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
@@ -86,8 +87,8 @@
8687

8788
import com.cloud.agent.AgentManager;
8889
import com.cloud.agent.api.Answer;
89-
import com.cloud.agent.api.CleanupSnapshotBackupCommand;
9090
import com.cloud.agent.api.Command;
91+
import com.cloud.agent.api.DeleteSnapshotBackupCommand;
9192
import com.cloud.agent.api.StoragePoolInfo;
9293
import com.cloud.agent.api.storage.DeleteTemplateCommand;
9394
import com.cloud.agent.api.storage.DeleteVolumeCommand;
@@ -1174,49 +1175,47 @@ public void cleanupSecondaryStorage(boolean recurring) {
11741175
}
11751176
}
11761177

1177-
// Cleanup snapshot in secondary storage hosts
1178+
// CleanUp snapshots on Secondary Storage.
11781179
for (DataStore store : imageStores) {
11791180
try {
1180-
List<Long> vIDs = findAllVolumeIdInSnapshotTable(store.getId());
1181-
if (vIDs == null) {
1182-
continue;
1183-
}
1184-
for (Long volumeId : vIDs) {
1185-
boolean lock = false;
1186-
try {
1187-
VolumeVO volume = _volsDao.findByIdIncludingRemoved(volumeId);
1188-
if (volume.getRemoved() == null) {
1189-
volume = _volsDao.acquireInLockTable(volumeId, 10);
1190-
if (volume == null) {
1191-
continue;
1192-
}
1193-
lock = true;
1194-
}
1195-
List<String> snapshots = findAllSnapshotForVolume(volumeId);
1196-
if (snapshots == null) {
1197-
continue;
1198-
}
1199-
EndPoint ep = _epSelector.select(store);
1200-
CleanupSnapshotBackupCommand cmd = new CleanupSnapshotBackupCommand(store.getUri(), store.getScope().getScopeId(),
1201-
volume.getAccountId(), volumeId, snapshots);
1181+
List<SnapshotDataStoreVO> destroyedSnapshotStoreVOs = _snapshotStoreDao.listDestroyed(store.getId());
1182+
s_logger.debug("Secondary storage garbage collector found " + destroyedSnapshotStoreVOs.size()
1183+
+ " snapshots to cleanup on secondary storage host: " + store.getName());
1184+
for (SnapshotDataStoreVO destroyedSnapshotStoreVO : destroyedSnapshotStoreVOs) {
1185+
// check if this snapshot has child
1186+
SnapshotInfo snap = snapshotFactory.getSnapshot(destroyedSnapshotStoreVO.getSnapshotId(), store);
1187+
if ( snap.getChild() != null ){
1188+
s_logger.debug("Skip snapshot on store: " + destroyedSnapshotStoreVO + " , because it has child");
1189+
continue;
1190+
}
1191+
1192+
if (s_logger.isDebugEnabled()) {
1193+
s_logger.debug("Deleting snapshot on store: " + destroyedSnapshotStoreVO);
1194+
}
12021195

1196+
String installPath = destroyedSnapshotStoreVO.getInstallPath();
1197+
1198+
if (installPath != null) {
1199+
EndPoint ep = _epSelector.select(store);
1200+
DeleteSnapshotBackupCommand cmd = new DeleteSnapshotBackupCommand(store.getTO(), store.getUri(),
1201+
null, null, null, destroyedSnapshotStoreVO.getInstallPath(), false);
12031202
Answer answer = ep.sendMessage(cmd);
1204-
if ((answer == null) || !answer.getResult()) {
1205-
String details = "Failed to cleanup snapshots for volume " + volumeId + " due to "
1206-
+ (answer == null ? "null" : answer.getDetails());
1207-
s_logger.warn(details);
1208-
}
1209-
} catch (Exception e1) {
1210-
s_logger.warn("problem cleaning up snapshots in secondary storage store " + store.getName(), e1);
1211-
} finally {
1212-
if (lock) {
1213-
_volsDao.releaseFromLockTable(volumeId);
1203+
if (answer == null || !answer.getResult()) {
1204+
s_logger.debug("Failed to delete " + destroyedSnapshotStoreVO + " due to "
1205+
+ ((answer == null) ? "answer is null" : answer.getDetails()));
1206+
} else {
1207+
_volumeStoreDao.remove(destroyedSnapshotStoreVO.getId());
1208+
s_logger.debug("Deleted snapshot at: " + destroyedSnapshotStoreVO.getInstallPath());
12141209
}
1210+
} else {
1211+
_snapshotStoreDao.remove(destroyedSnapshotStoreVO.getId());
12151212
}
12161213
}
1214+
12171215
} catch (Exception e2) {
12181216
s_logger.warn("problem cleaning up snapshots in secondary storage store " + store.getName(), e2);
12191217
}
1218+
12201219
}
12211220

12221221
// CleanUp volumes on Secondary Storage.

0 commit comments

Comments
 (0)