Change vm.stats.remove.batch.size to delete.batch.query.size & allow delete of volume_stats in batches

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

@@ -75,8 +75,10 @@ public interface VolumeStatsDao extends GenericDao<VolumeStatsVO, Long> {
     /**
      * 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);
 
 }

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<VolumeStatsVO, Long> implements VolumeStatsDao {
 
+    protected Logger logger = Logger.getLogger(getClass());
+
     protected SearchBuilder<VolumeStatsVO> volumeIdSearch;
     protected SearchBuilder<VolumeStatsVO> volumeIdTimestampGreaterThanEqualSearch;
     protected SearchBuilder<VolumeStatsVO> volumeIdTimestampLessThanEqualSearch;
@@ -116,9 +119,21 @@ public void removeAllByVolumeId(long volumeId) {
     }
 
     @Override
-    public void removeAllByTimestampLessThan(Date limit) {
+    public void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery) {
         SearchCriteria<VolumeStatsVO> 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));
     }
 }

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -506,6 +506,11 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> 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<Long> 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);
+
     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_QUERY_BATCH_SIZE
         };
     }
 

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_QUERY_BATCH_SIZE;
 import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
 
 import java.lang.management.ManagementFactory;
@@ -284,11 +285,6 @@ public String toString() {
     protected static ConfigKey<Boolean> 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<Long> 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<Boolean> 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_QUERY_BATCH_SIZE.value());
     }
 
     /**
@@ -1986,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_QUERY_BATCH_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,