From e562cb0bb75faeefec6fb120a41c6a88abd523bd Mon Sep 17 00:00:00 2001 From: Vishesh Date: Sat, 22 Jun 2024 01:11:15 +0530 Subject: [PATCH 1/6] Change vm.stats.remove.batch.size to delete.batch.query.size --- .../cloud/configuration/ConfigurationManagerImpl.java | 8 +++++++- .../src/main/java/com/cloud/server/StatsCollector.java | 10 +++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index eb72a626036c..9595f757de6f 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -506,6 +506,11 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static final ConfigKey ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS = new ConfigKey<>(Boolean.class, "allow.domain.admins.to.create.tagged.offerings", "Advanced", "false", "Allow domain admins to create offerings with tags.", true, ConfigKey.Scope.Account, null); + public static final ConfigKey DELETE_BATCH_QUERY_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.batch.query.size", "0", + "Indicates the limit applied while deleting entries in bulk. With this, ACS will run the delete query, applying the limit, as many times as necessary" + + " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. Zero (0) means that no limit will be applied, " + + "therefore, the query will run once and without limit, keeping the default behavior. For now, this is used for deletion of vm stats only.", true); + private static final String IOPS_READ_RATE = "IOPS Read"; private static final String IOPS_WRITE_RATE = "IOPS Write"; private static final String BYTES_READ_RATE = "Bytes Read"; @@ -7797,7 +7802,8 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE, VM_SERVICE_OFFERING_MAX_CPU_CORES, VM_SERVICE_OFFERING_MAX_RAM_SIZE, MIGRATE_VM_ACROSS_CLUSTERS, - ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN, ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS + ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN, + ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_BATCH_QUERY_SIZE }; } diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 070508f2502e..65a64da74eae 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.server; +import static com.cloud.configuration.ConfigurationManagerImpl.DELETE_BATCH_QUERY_SIZE; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import java.lang.management.ManagementFactory; @@ -284,11 +285,6 @@ public String toString() { protected static ConfigKey vmStatsCollectUserVMOnly = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.user.vm.only", "false", "When set to 'false' stats for system VMs will be collected otherwise stats collection will be done only for user VMs", true); - protected static ConfigKey vmStatsRemoveBatchSize = new ConfigKey<>("Advanced", Long.class, "vm.stats.remove.batch.size", "0", "Indicates the" + - " limit applied to delete vm_stats entries while running the clean-up task. With this, ACS will run the delete query, applying the limit, as many times as necessary" + - " to delete all entries older than the value defined in vm.stats.max.retention.time. This is advised when retaining several days of records, which can lead to slowness" + - " on the delete query. Zero (0) means that no limit will be applied, therefore, the query will run once and without limit, keeping the default behavior.", true); - protected static ConfigKey vmDiskStatsRetentionEnabled = new ConfigKey<>("Advanced", Boolean.class, "vm.disk.stats.retention.enabled", "false", "When set to 'true' stats for VM disks will be stored in the database otherwise disk stats will not be stored", true); @@ -1967,7 +1963,7 @@ protected void cleanUpVirtualMachineStats() { LOGGER.trace("Removing older VM stats records."); Date now = new Date(); Date limit = DateUtils.addMinutes(now, -maxRetentionTime); - vmStatsDao.removeAllByTimestampLessThan(limit, vmStatsRemoveBatchSize.value()); + vmStatsDao.removeAllByTimestampLessThan(limit, DELETE_BATCH_QUERY_SIZE.value()); } /** @@ -2141,7 +2137,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, vmStatsRemoveBatchSize, + return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime, MANAGEMENT_SERVER_STATUS_COLLECTION_INTERVAL, DATABASE_SERVER_STATUS_COLLECTION_INTERVAL, From dea2467057b7f2369668e2b7bd265baa585766d1 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 24 Jun 2024 18:21:45 +0530 Subject: [PATCH 2/6] Add support for deletion of volume stats in batches --- .../com/cloud/storage/dao/VolumeStatsDao.java | 6 ++++-- .../cloud/storage/dao/VolumeStatsDaoImpl.java | 21 ++++++++++++++++--- .../ConfigurationManagerImpl.java | 2 +- .../java/com/cloud/server/StatsCollector.java | 2 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDao.java index ff6af56c9c30..b4a596dfc8d5 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDao.java @@ -75,8 +75,10 @@ public interface VolumeStatsDao extends GenericDao { /** * Removes (expunges) all Volume stats with {@code timestamp} less than * a given Date. - * @param limit the maximum date to keep stored. Records that exceed this limit will be removed. + * @param limitDate the maximum date to keep stored. Records that exceed this limit will be removed. + * @param limitPerQuery the maximum amount of rows to be removed in a single query. We loop if there are still rows to be removed after a given query. + * If 0 or negative, no limit is used. */ - void removeAllByTimestampLessThan(Date limit); + void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java index 5d0d3c8921cf..002310125af6 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeStatsDaoImpl.java @@ -21,6 +21,7 @@ import javax.annotation.PostConstruct; +import org.apache.log4j.Logger; import org.springframework.stereotype.Component; import com.cloud.utils.db.Filter; @@ -33,6 +34,8 @@ @Component public class VolumeStatsDaoImpl extends GenericDaoBase implements VolumeStatsDao { + protected Logger logger = Logger.getLogger(getClass()); + protected SearchBuilder volumeIdSearch; protected SearchBuilder volumeIdTimestampGreaterThanEqualSearch; protected SearchBuilder volumeIdTimestampLessThanEqualSearch; @@ -116,9 +119,21 @@ public void removeAllByVolumeId(long volumeId) { } @Override - public void removeAllByTimestampLessThan(Date limit) { + public void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery) { SearchCriteria sc = timestampSearch.create(); - sc.setParameters(TIMESTAMP, limit); - expunge(sc); + sc.setParameters(TIMESTAMP, limitDate); + + logger.debug(String.format("Starting to remove all volume_stats rows older than [%s].", limitDate)); + + long totalRemoved = 0; + long removed; + + do { + removed = expunge(sc, limitPerQuery); + totalRemoved += removed; + logger.trace(String.format("Removed [%s] volume_stats rows on the last update and a sum of [%s] volume_stats rows older than [%s] until now.", removed, totalRemoved, limitDate)); + } while (limitPerQuery > 0 && removed >= limitPerQuery); + + logger.info(String.format("Removed a total of [%s] volume_stats rows older than [%s].", totalRemoved, limitDate)); } } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 9595f757de6f..516708077182 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -509,7 +509,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static final ConfigKey DELETE_BATCH_QUERY_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.batch.query.size", "0", "Indicates the limit applied while deleting entries in bulk. With this, ACS will run the delete query, applying the limit, as many times as necessary" + " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. Zero (0) means that no limit will be applied, " + - "therefore, the query will run once and without limit, keeping the default behavior. For now, this is used for deletion of vm stats only.", true); + "therefore, the query will run once and without limit, keeping the default behavior. For now, this is used for deletion of vm & volume stats only.", true); private static final String IOPS_READ_RATE = "IOPS Read"; private static final String IOPS_WRITE_RATE = "IOPS Write"; diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 65a64da74eae..f342733a53cb 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1982,7 +1982,7 @@ protected void cleanUpVolumeStats() { LOGGER.trace("Removing older Volume stats records."); Date now = new Date(); Date limit = DateUtils.addMinutes(now, -maxRetentionTime); - volumeStatsDao.removeAllByTimestampLessThan(limit); + volumeStatsDao.removeAllByTimestampLessThan(limit, DELETE_BATCH_QUERY_SIZE.value()); } /** From 8b39a51720b5599029b0b23249b7764bf9ad64d1 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 27 Jun 2024 14:35:52 +0530 Subject: [PATCH 3/6] Update server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Co-authored-by: Suresh Kumar Anaparti --- .../java/com/cloud/configuration/ConfigurationManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 516708077182..d5d90718581f 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -507,7 +507,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati "false", "Allow domain admins to create offerings with tags.", true, ConfigKey.Scope.Account, null); public static final ConfigKey DELETE_BATCH_QUERY_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.batch.query.size", "0", - "Indicates the limit applied while deleting entries in bulk. With this, ACS will run the delete query, applying the limit, as many times as necessary" + + "Indicates the limit applied while deleting entries in bulk. With this, the delete query will apply the limit as many times as necessary," + " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. Zero (0) means that no limit will be applied, " + "therefore, the query will run once and without limit, keeping the default behavior. For now, this is used for deletion of vm & volume stats only.", true); From 9d89f17ee5ea3e1d62dcbb29fdabd268c47dc6dc Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 27 Jun 2024 14:38:19 +0530 Subject: [PATCH 4/6] Update server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Co-authored-by: Suresh Kumar Anaparti --- .../java/com/cloud/configuration/ConfigurationManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index d5d90718581f..46eb2ce27032 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -509,7 +509,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static final ConfigKey DELETE_BATCH_QUERY_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.batch.query.size", "0", "Indicates the limit applied while deleting entries in bulk. With this, the delete query will apply the limit as many times as necessary," + " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. Zero (0) means that no limit will be applied, " + - "therefore, the query will run once and without limit, keeping the default behavior. For now, this is used for deletion of vm & volume stats only.", true); + "therefore, the query will run once and without limit, this is default value which keeps the current behavior. For now, this is used for deletion of vm & volume stats only.", true); private static final String IOPS_READ_RATE = "IOPS Read"; private static final String IOPS_WRITE_RATE = "IOPS Write"; From beea194894d92bf822c8036202bba9570e0abdcc Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 27 Jun 2024 15:01:53 +0530 Subject: [PATCH 5/6] Update configkey description --- .../com/cloud/configuration/ConfigurationManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 46eb2ce27032..1564cc4e4592 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -508,8 +508,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static final ConfigKey DELETE_BATCH_QUERY_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.batch.query.size", "0", "Indicates the limit applied while deleting entries in bulk. With this, the delete query will apply the limit as many times as necessary," + - " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. Zero (0) means that no limit will be applied, " + - "therefore, the query will run once and without limit, this is default value which keeps the current behavior. For now, this is used for deletion of vm & volume stats only.", true); + " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. <= 0 means that no limit will " + + "be applied. Default value is 0. For now, this is used for deletion of vm & volume stats only.", true); private static final String IOPS_READ_RATE = "IOPS Read"; private static final String IOPS_WRITE_RATE = "IOPS Write"; From 745b212f125a67fc081ec80206304feb8c238693 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 27 Jun 2024 19:29:49 +0530 Subject: [PATCH 6/6] Address comments --- .../com/cloud/configuration/ConfigurationManagerImpl.java | 4 ++-- server/src/main/java/com/cloud/server/StatsCollector.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 1564cc4e4592..64982af91e7e 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -506,7 +506,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static final ConfigKey ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS = new ConfigKey<>(Boolean.class, "allow.domain.admins.to.create.tagged.offerings", "Advanced", "false", "Allow domain admins to create offerings with tags.", true, ConfigKey.Scope.Account, null); - public static final ConfigKey DELETE_BATCH_QUERY_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.batch.query.size", "0", + public static final ConfigKey DELETE_QUERY_BATCH_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.query.batch.size", "0", "Indicates the limit applied while deleting entries in bulk. With this, the delete query will apply the limit as many times as necessary," + " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. <= 0 means that no limit will " + "be applied. Default value is 0. For now, this is used for deletion of vm & volume stats only.", true); @@ -7803,7 +7803,7 @@ public ConfigKey[] getConfigKeys() { BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE, VM_SERVICE_OFFERING_MAX_CPU_CORES, VM_SERVICE_OFFERING_MAX_RAM_SIZE, MIGRATE_VM_ACROSS_CLUSTERS, ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN, - ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_BATCH_QUERY_SIZE + ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_QUERY_BATCH_SIZE }; } diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index f342733a53cb..308ca11f469f 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -16,7 +16,7 @@ // under the License. package com.cloud.server; -import static com.cloud.configuration.ConfigurationManagerImpl.DELETE_BATCH_QUERY_SIZE; +import static com.cloud.configuration.ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import java.lang.management.ManagementFactory; @@ -1963,7 +1963,7 @@ protected void cleanUpVirtualMachineStats() { LOGGER.trace("Removing older VM stats records."); Date now = new Date(); Date limit = DateUtils.addMinutes(now, -maxRetentionTime); - vmStatsDao.removeAllByTimestampLessThan(limit, DELETE_BATCH_QUERY_SIZE.value()); + vmStatsDao.removeAllByTimestampLessThan(limit, DELETE_QUERY_BATCH_SIZE.value()); } /** @@ -1982,7 +1982,7 @@ protected void cleanUpVolumeStats() { LOGGER.trace("Removing older Volume stats records."); Date now = new Date(); Date limit = DateUtils.addMinutes(now, -maxRetentionTime); - volumeStatsDao.removeAllByTimestampLessThan(limit, DELETE_BATCH_QUERY_SIZE.value()); + volumeStatsDao.removeAllByTimestampLessThan(limit, DELETE_QUERY_BATCH_SIZE.value()); } /**