From 9077c9a5b47224b2d00978ff67e9af6957ec589b Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 3 Nov 2015 17:04:55 +0100 Subject: [PATCH 1/2] CLOUDSTACK-9022: keep Destroyed volumes for sometime --- engine/schema/src/com/cloud/storage/dao/VolumeDao.java | 3 +++ .../src/com/cloud/storage/dao/VolumeDaoImpl.java | 10 ++++++++++ server/src/com/cloud/configuration/Config.java | 1 + server/src/com/cloud/storage/StorageManagerImpl.java | 9 ++++++--- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/engine/schema/src/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/com/cloud/storage/dao/VolumeDao.java index 4959ce4d63b..a05dc1f560d 100644 --- a/engine/schema/src/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/com/cloud/storage/dao/VolumeDao.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.storage.dao; +import java.util.Date; import java.util.List; import com.cloud.hypervisor.Hypervisor.HypervisorType; @@ -79,6 +80,8 @@ public interface VolumeDao extends GenericDao, StateDao listVolumesToBeDestroyed(); + List listVolumesToBeDestroyed(Date date); + ImageFormat getImageFormat(Long volumeId); List findReadyRootVolumesByInstance(long instanceId); diff --git a/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java index f5738477540..d53471fee5c 100644 --- a/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java @@ -331,6 +331,7 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol AllFieldsSearch.and("id", AllFieldsSearch.entity().getId(), Op.EQ); AllFieldsSearch.and("destroyed", AllFieldsSearch.entity().getState(), Op.EQ); AllFieldsSearch.and("notDestroyed", AllFieldsSearch.entity().getState(), Op.NEQ); + AllFieldsSearch.and("updateTime", AllFieldsSearch.entity().getUpdated(), SearchCriteria.Op.LT); AllFieldsSearch.and("updatedCount", AllFieldsSearch.entity().getUpdatedCount(), Op.EQ); AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ); AllFieldsSearch.done(); @@ -482,6 +483,15 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol return listBy(sc); } + @Override + public List listVolumesToBeDestroyed(Date date) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("state", Volume.State.Destroy); + sc.setParameters("updateTime", date); + + return listBy(sc); + } + @Override public boolean updateState(com.cloud.storage.Volume.State currentState, Event event, com.cloud.storage.Volume.State nextState, Volume vo, Object data) { diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index 182ec50e300..d9abecde86e 100644 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -672,6 +672,7 @@ public enum Config { "86400", "The interval (in seconds) to wait before running the storage cleanup thread.", null), + StorageCleanupDelay("Advanced", StorageManager.class, Integer.class, "storage.cleanup.delay", "86400", "Determines how long (in seconds) to wait before actually expunging destroyed volumes. The default value = the default value of storage.cleanup.interval.", null), StorageCleanupEnabled("Advanced", StorageManager.class, Boolean.class, "storage.cleanup.enabled", "true", "Enables/disables the storage cleanup thread.", null), UpdateWait("Advanced", AgentManager.class, Integer.class, "update.wait", "600", "Time to wait (in seconds) before alerting on a updating agent", null), XapiWait("Advanced", AgentManager.class, Integer.class, "xapiwait", "60", "Time (in seconds) to wait for XAPI to return", null), diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index ba39e1f0fa8..595cf145b3e 100644 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -301,6 +301,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C boolean _templateCleanupEnabled = true; int _storageCleanupInterval; int _storagePoolAcquisitionWaitSeconds = 1800; // 30 minutes + int _storageCleanupDelay; int _downloadUrlCleanupInterval; int _downloadUrlExpirationInterval; // protected BigDecimal _overProvisioningFactor = new BigDecimal(1); @@ -470,9 +471,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C String time = configs.get("storage.cleanup.interval"); _storageCleanupInterval = NumbersUtil.parseInt(time, 86400); + time = configs.get("storage.cleanup.delay"); + _storageCleanupDelay = NumbersUtil.parseInt(time, _storageCleanupInterval); - s_logger.info("Storage cleanup enabled: " + _storageCleanupEnabled + ", interval: " + _storageCleanupInterval + ", template cleanup enabled: " + - _templateCleanupEnabled); + s_logger.info("Storage cleanup enabled: " + _storageCleanupEnabled + ", interval: " + _storageCleanupInterval + ", delay: " + _storageCleanupDelay + + ", template cleanup enabled: " + _templateCleanupEnabled); String cleanupInterval = configs.get("extract.url.cleanup.interval"); _downloadUrlCleanupInterval = NumbersUtil.parseInt(cleanupInterval, 7200); @@ -1111,7 +1114,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C cleanupSecondaryStorage(recurring); - List vols = _volsDao.listVolumesToBeDestroyed(); + List vols = _volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) _storageCleanupDelay << 10))); for (VolumeVO vol : vols) { try { volService.expungeVolumeAsync(volFactory.getVolume(vol.getId())); From 4ed1e0d5f809d1bb2845b6e076d55eeb1ce3f0f4 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 4 Nov 2015 09:27:32 +0100 Subject: [PATCH 2/2] CLOUDSTACK-9022: move storage.cleanup related global configurations to StorageManager --- .../src/com/cloud/configuration/Config.java | 10 ----- .../src/com/cloud/storage/StorageManager.java | 8 ++++ .../com/cloud/storage/StorageManagerImpl.java | 37 ++++++++++--------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index d9abecde86e..f13ebf58f32 100644 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -664,16 +664,6 @@ public enum Config { "600", "Time in seconds between retries to stop or destroy a vm", null), - StorageCleanupInterval( - "Advanced", - StorageManager.class, - Integer.class, - "storage.cleanup.interval", - "86400", - "The interval (in seconds) to wait before running the storage cleanup thread.", - null), - StorageCleanupDelay("Advanced", StorageManager.class, Integer.class, "storage.cleanup.delay", "86400", "Determines how long (in seconds) to wait before actually expunging destroyed volumes. The default value = the default value of storage.cleanup.interval.", null), - StorageCleanupEnabled("Advanced", StorageManager.class, Boolean.class, "storage.cleanup.enabled", "true", "Enables/disables the storage cleanup thread.", null), UpdateWait("Advanced", AgentManager.class, Integer.class, "update.wait", "600", "Time to wait (in seconds) before alerting on a updating agent", null), XapiWait("Advanced", AgentManager.class, Integer.class, "xapiwait", "60", "Time (in seconds) to wait for XAPI to return", null), MigrateWait("Advanced", AgentManager.class, Integer.class, "migratewait", "3600", "Time (in seconds) to wait for VM migrate finish", null), diff --git a/server/src/com/cloud/storage/StorageManager.java b/server/src/com/cloud/storage/StorageManager.java index 5487e2ec17f..a399a083f7c 100644 --- a/server/src/com/cloud/storage/StorageManager.java +++ b/server/src/com/cloud/storage/StorageManager.java @@ -21,6 +21,7 @@ import java.util.List; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import com.cloud.agent.api.Answer; @@ -39,6 +40,13 @@ import com.cloud.utils.Pair; import com.cloud.vm.VMInstanceVO; public interface StorageManager extends StorageService { + static final ConfigKey StorageCleanupInterval = new ConfigKey(Integer.class, "storage.cleanup.interval", "Advanced", "86400", + "The interval (in seconds) to wait before running the storage cleanup thread.", false, ConfigKey.Scope.Global, null); + static final ConfigKey StorageCleanupDelay = new ConfigKey(Integer.class, "storage.cleanup.delay", "Advanced", "86400", + "Determines how long (in seconds) to wait before actually expunging destroyed volumes. The default value = the default value of storage.cleanup.interval.", false, ConfigKey.Scope.Global, null); + static final ConfigKey StorageCleanupEnabled = new ConfigKey(Boolean.class, "storage.cleanup.enabled", "Advanced", "true", + "Enables/disables the storage cleanup thread.", false, ConfigKey.Scope.Global, null); + /** * Returns a comma separated list of tags for the specified storage pool * @param poolId diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 595cf145b3e..b8890bfc0af 100644 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -79,6 +79,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; import org.apache.cloudstack.framework.async.AsyncCallFuture; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; @@ -190,7 +192,7 @@ import com.cloud.vm.dao.VMInstanceDao; @Component @Local(value = {StorageManager.class, StorageService.class}) -public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener { +public class StorageManagerImpl extends ManagerBase implements StorageManager, ClusterManagerListener, Configurable { private static final Logger s_logger = Logger.getLogger(StorageManagerImpl.class); protected String _name; @@ -297,11 +299,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C protected SearchBuilder LocalStorageSearch; ScheduledExecutorService _executor = null; - boolean _storageCleanupEnabled; boolean _templateCleanupEnabled = true; - int _storageCleanupInterval; int _storagePoolAcquisitionWaitSeconds = 1800; // 30 minutes - int _storageCleanupDelay; int _downloadUrlCleanupInterval; int _downloadUrlExpirationInterval; // protected BigDecimal _overProvisioningFactor = new BigDecimal(1); @@ -463,18 +462,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C _agentMgr.registerForHostEvents(new StoragePoolMonitor(this, _storagePoolDao), true, false, true); - String storageCleanupEnabled = configs.get("storage.cleanup.enabled"); - _storageCleanupEnabled = (storageCleanupEnabled == null) ? true : Boolean.parseBoolean(storageCleanupEnabled); - String value = _configDao.getValue(Config.StorageTemplateCleanupEnabled.key()); _templateCleanupEnabled = (value == null ? true : Boolean.parseBoolean(value)); - String time = configs.get("storage.cleanup.interval"); - _storageCleanupInterval = NumbersUtil.parseInt(time, 86400); - time = configs.get("storage.cleanup.delay"); - _storageCleanupDelay = NumbersUtil.parseInt(time, _storageCleanupInterval); - - s_logger.info("Storage cleanup enabled: " + _storageCleanupEnabled + ", interval: " + _storageCleanupInterval + ", delay: " + _storageCleanupDelay + + s_logger.info("Storage cleanup enabled: " + StorageCleanupEnabled.value() + ", interval: " + StorageCleanupInterval.value() + ", delay: " + StorageCleanupDelay.value() + ", template cleanup enabled: " + _templateCleanupEnabled); String cleanupInterval = configs.get("extract.url.cleanup.interval"); @@ -528,10 +519,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public boolean start() { - if (_storageCleanupEnabled) { + if (StorageCleanupEnabled.value()) { Random generator = new Random(); - int initialDelay = generator.nextInt(_storageCleanupInterval); - _executor.scheduleWithFixedDelay(new StorageGarbageCollector(), initialDelay, _storageCleanupInterval, TimeUnit.SECONDS); + int initialDelay = generator.nextInt(StorageCleanupInterval.value()); + _executor.scheduleWithFixedDelay(new StorageGarbageCollector(), initialDelay, StorageCleanupInterval.value(), TimeUnit.SECONDS); } else { s_logger.debug("Storage cleanup is not enabled, so the storage cleanup thread is not being scheduled."); } @@ -543,7 +534,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public boolean stop() { - if (_storageCleanupEnabled) { + if (StorageCleanupEnabled.value()) { _executor.shutdown(); } return true; @@ -1114,7 +1105,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C cleanupSecondaryStorage(recurring); - List vols = _volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) _storageCleanupDelay << 10))); + List vols = _volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) StorageCleanupDelay.value() << 10))); for (VolumeVO vol : vols) { try { volService.expungeVolumeAsync(volFactory.getVolume(vol.getId())); @@ -2306,4 +2297,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } return 0L; } + + @Override + public String getConfigComponentName() { + return StorageManager.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {StorageCleanupInterval, StorageCleanupDelay, StorageCleanupEnabled}; + } }