hsato03 commented on code in PR #8031:
URL: https://github.com/apache/cloudstack/pull/8031#discussion_r1353106386
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -1289,37 +1289,52 @@ public void cleanupStorage(boolean recurring) {
try {
List<VMTemplateStoragePoolVO>
unusedTemplatesInPool = _tmpltMgr.getUnusedTemplatesInPool(pool);
- s_logger.debug("Storage pool garbage collector
found " + unusedTemplatesInPool.size() + " templates to clean up in storage
pool: " + pool.getName());
+ s_logger.debug(String.format("Storage pool
garbage collector found [%s] templates to be cleaned up in storage pool [%s].",
unusedTemplatesInPool.size(), pool.getName()));
for (VMTemplateStoragePoolVO templatePoolVO :
unusedTemplatesInPool) {
if (templatePoolVO.getDownloadState() !=
VMTemplateStorageResourceAssoc.Status.DOWNLOADED) {
- s_logger.debug("Storage pool garbage
collector is skipping template with ID: " + templatePoolVO.getTemplateId() + "
on pool " + templatePoolVO.getPoolId()
- + " because it is not completely
downloaded.");
+ s_logger.debug(String.format("Storage
pool garbage collector is skipping template [%s] clean up on pool [%s] " +
+ "because it is not completely
downloaded.", templatePoolVO.getTemplateId(), templatePoolVO.getPoolId()));
continue;
}
if (!templatePoolVO.getMarkedForGC()) {
templatePoolVO.setMarkedForGC(true);
_vmTemplatePoolDao.update(templatePoolVO.getId(), templatePoolVO);
- s_logger.debug("Storage pool garbage
collector has marked template with ID: " + templatePoolVO.getTemplateId() + "
on pool " + templatePoolVO.getPoolId()
- + " for garbage collection.");
+ s_logger.debug(String.format("Storage
pool garbage collector has marked template [%s] on pool [%s] " +
+ "for garbage collection.",
templatePoolVO.getTemplateId(), templatePoolVO.getPoolId()));
continue;
}
_tmpltMgr.evictTemplateFromStoragePool(templatePoolVO);
}
} catch (Exception e) {
- s_logger.warn("Problem cleaning up primary
storage pool " + pool, e);
+ s_logger.error(String.format("Failed to clean
up primary storage pool [%s] due to: [%s].", pool, e.getMessage()), e);
}
}
}
//destroy snapshots in destroying state in
snapshot_store_ref
List<SnapshotDataStoreVO> ssSnapshots =
_snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying);
for (SnapshotDataStoreVO ssSnapshotVO : ssSnapshots) {
+ SnapshotVO snapshot =
_snapshotDao.findById(ssSnapshotVO.getSnapshotId());
+ if (snapshot == null) {
+ s_logger.warn(String.format("Did not find snapshot
[%s] in destroying state; therefore, it cannot be destroyed.",
ssSnapshotVO.getSnapshotId()));
+ continue;
+ }
+
+ String snapshotUuid = snapshot.getUuid();
try {
-
_snapshotService.deleteSnapshot(snapshotFactory.getSnapshot(ssSnapshotVO.getSnapshotId(),
DataStoreRole.Image));
+ s_logger.debug(String.format("Verifying if
snapshot [%s] is in destroying state in any image data store.", snapshotUuid));
+ SnapshotInfo snapshotInfo =
snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Image);
+
+ if (snapshotInfo != null) {
Review Comment:
You are right. But, `snapshotInfo` is getting only snapshots from Image
data store role. If a snapshot is in destroying state in a Primary data store
role and the same snapshot in the Image data store role has already been
cleaned up, `snapshotInfo` will be null and a NPE will be thrown.
Snapshots from ID 18 & 19 were cleaned up in Image data store but the
snapshot still remains in destroying state in Primary data store.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
