From a2f238ba6a281ace37627480ba164be866dbfc81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= Date: Thu, 22 Feb 2024 23:30:51 -0300 Subject: [PATCH 1/4] Add configuration to limit the number of rows deleted from vm_stats --- .../java/com/cloud/vm/dao/VmStatsDao.java | 5 +++-- .../java/com/cloud/vm/dao/VmStatsDaoImpl.java | 21 ++++++++++++++++--- .../java/com/cloud/utils/db/GenericDao.java | 8 +++++++ .../com/cloud/utils/db/GenericDaoBase.java | 12 ++++++++++- .../java/com/cloud/server/StatsCollector.java | 12 ++++++----- .../com/cloud/server/StatsCollectorTest.java | 4 ++-- .../com/cloud/user/MockUsageEventDao.java | 5 +++++ 7 files changed, 54 insertions(+), 13 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java index 879faaf5c901..980fec2cc542 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java @@ -75,8 +75,9 @@ public interface VmStatsDao extends GenericDao { /** * Removes (expunges) all VM 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. */ - void removeAllByTimestampLessThan(Date limit); + void removeAllByTimestampLessThan(Date limitDate, Long limitPerQuery); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java index 1bef8f0626c3..ea70a185cb5e 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.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 VmStatsDaoImpl extends GenericDaoBase implements VmStatsDao { + protected Logger logger = Logger.getLogger(getClass()); + protected SearchBuilder vmIdSearch; protected SearchBuilder vmIdTimestampGreaterThanEqualSearch; protected SearchBuilder vmIdTimestampLessThanEqualSearch; @@ -113,10 +116,22 @@ public void removeAllByVmId(long vmId) { } @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 vm_stats rows older than [%s].", limitDate)); + + long totalRemoved = 0; + long removed; + + do { + removed = expunge(sc, limitPerQuery); + totalRemoved += removed; + logger.trace(String.format("Removed [%s] vm_stats rows on the last update and a sum of [%s] vm_stats rows older than [%s] until now.", removed, totalRemoved, limitDate)); + } while (limitPerQuery > 0 && removed >= limitPerQuery); + + logger.info(String.format("Removed a total of [%s] vm_stats rows older than [%s].", totalRemoved, limitDate)); } } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java index b2a9e6f733aa..1bd7569c593f 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java @@ -229,6 +229,14 @@ public interface GenericDao { */ int expunge(final SearchCriteria sc); + /** + * Delete the entity beans specified by the search criteria with a given limit + * @param sc Search criteria + * @param limit Maximum number of rows that will be affected + * @return Number of rows deleted + */ + int expunge(SearchCriteria sc, long limit); + /** * expunge the removed rows. */ diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 6bf36df09414..d3f6618714e6 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1208,9 +1208,14 @@ public boolean expunge(final ID id) { } } - // FIXME: Does not work for joins. @Override public int expunge(final SearchCriteria sc) { + return expunge(sc, -1); + } + + // FIXME: Does not work for joins. + @Override + public int expunge(final SearchCriteria sc, long limit) { if (sc == null) { throw new CloudRuntimeException("Call to throw new expunge with null search Criteria"); } @@ -1223,6 +1228,11 @@ public int expunge(final SearchCriteria sc) { str.append(sc.getWhereClause()); } + if (limit > 0) { + str.append(" LIMIT "); + str.append(limit); + } + final String sql = str.toString(); final TransactionLegacy txn = TransactionLegacy.currentTxn(); diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 2467416155a1..9abf584d6381 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -114,9 +114,6 @@ import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; import com.cloud.serializer.GsonHelper; -import com.cloud.server.StatsCollector.AbstractStatsCollector; -import com.cloud.server.StatsCollector.AutoScaleMonitor; -import com.cloud.server.StatsCollector.StorageCollector; import com.cloud.storage.ImageStoreDetailsUtil; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; @@ -290,6 +287,11 @@ 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); @@ -1955,7 +1957,7 @@ protected void cleanUpVirtualMachineStats() { LOGGER.trace("Removing older VM stats records."); Date now = new Date(); Date limit = DateUtils.addMinutes(now, -maxRetentionTime); - vmStatsDao.removeAllByTimestampLessThan(limit); + vmStatsDao.removeAllByTimestampLessThan(limit, vmStatsRemoveBatchSize.value()); } /** @@ -2130,7 +2132,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, - vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime, + vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime, vmStatsRemoveBatchSize, VM_STATS_INCREMENT_METRICS_IN_MEMORY, MANAGEMENT_SERVER_STATUS_COLLECTION_INTERVAL, DATABASE_SERVER_STATUS_COLLECTION_INTERVAL, diff --git a/server/src/test/java/com/cloud/server/StatsCollectorTest.java b/server/src/test/java/com/cloud/server/StatsCollectorTest.java index 1f6a35cfbae2..1338c53453f1 100644 --- a/server/src/test/java/com/cloud/server/StatsCollectorTest.java +++ b/server/src/test/java/com/cloud/server/StatsCollectorTest.java @@ -304,7 +304,7 @@ public void cleanUpVirtualMachineStatsTestIsDisabled() { statsCollector.cleanUpVirtualMachineStats(); - Mockito.verify(vmStatsDaoMock, Mockito.never()).removeAllByTimestampLessThan(Mockito.any()); + Mockito.verify(vmStatsDaoMock, Mockito.never()).removeAllByTimestampLessThan(Mockito.any(), Mockito.any()); } @Test @@ -313,7 +313,7 @@ public void cleanUpVirtualMachineStatsTestIsEnabled() { statsCollector.cleanUpVirtualMachineStats(); - Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any()); + Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any(), Mockito.any()); } @Test diff --git a/server/src/test/java/com/cloud/user/MockUsageEventDao.java b/server/src/test/java/com/cloud/user/MockUsageEventDao.java index 4639c5092490..67ace2b8a71e 100644 --- a/server/src/test/java/com/cloud/user/MockUsageEventDao.java +++ b/server/src/test/java/com/cloud/user/MockUsageEventDao.java @@ -210,6 +210,11 @@ public int expunge(SearchCriteria sc) { return 0; } + @Override + public int expunge(SearchCriteria sc, long limit) { + return 0; + } + @Override public void expunge() { From ef39209c2f9b11d575de7ea565a4e7ea9e030800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 31 May 2024 14:37:37 -0300 Subject: [PATCH 2/4] Address review --- engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java | 3 ++- .../schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java index 980fec2cc542..0d7aa703a8cb 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java @@ -77,7 +77,8 @@ public interface VmStatsDao extends GenericDao { * a given Date. * @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 limitDate, Long limitPerQuery); + void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java index ea70a185cb5e..a98302e21360 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java @@ -116,7 +116,7 @@ public void removeAllByVmId(long vmId) { } @Override - public void removeAllByTimestampLessThan(Date limitDate, Long limitPerQuery) { + public void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery) { SearchCriteria sc = timestampSearch.create(); sc.setParameters("timestamp", limitDate); From 81bb6095d15565de4d2c0263513be6db3336df7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Wed, 12 Jun 2024 09:17:37 -0300 Subject: [PATCH 3/4] fix tests --- .../src/test/java/com/cloud/server/StatsCollectorTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/com/cloud/server/StatsCollectorTest.java b/server/src/test/java/com/cloud/server/StatsCollectorTest.java index 1338c53453f1..9f428d5d8e0c 100644 --- a/server/src/test/java/com/cloud/server/StatsCollectorTest.java +++ b/server/src/test/java/com/cloud/server/StatsCollectorTest.java @@ -29,6 +29,7 @@ import com.cloud.vm.VmStats; import com.cloud.vm.VmStatsVO; import com.cloud.vm.dao.VmStatsDao; +import com.cloud.vm.dao.VmStatsDaoImpl; import com.google.gson.Gson; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; @@ -81,7 +82,7 @@ public class StatsCollectorTest { private static final String DEFAULT_DATABASE_NAME = "cloudstack"; @Mock - VmStatsDao vmStatsDaoMock = Mockito.mock(VmStatsDao.class); + VmStatsDaoImpl vmStatsDaoMock; @Mock VmStatsEntry statsForCurrentIterationMock; @@ -304,7 +305,7 @@ public void cleanUpVirtualMachineStatsTestIsDisabled() { statsCollector.cleanUpVirtualMachineStats(); - Mockito.verify(vmStatsDaoMock, Mockito.never()).removeAllByTimestampLessThan(Mockito.any(), Mockito.any()); + Mockito.verify(vmStatsDaoMock, Mockito.never()).removeAllByTimestampLessThan(Mockito.any(), Mockito.anyLong()); } @Test @@ -313,7 +314,7 @@ public void cleanUpVirtualMachineStatsTestIsEnabled() { statsCollector.cleanUpVirtualMachineStats(); - Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any(), Mockito.any()); + Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any(), Mockito.anyLong()); } @Test From c47ddc603068b3323a1f42d5d77f37568b88625f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Wed, 12 Jun 2024 11:44:18 -0300 Subject: [PATCH 4/4] fix checkstyle --- server/src/test/java/com/cloud/server/StatsCollectorTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/com/cloud/server/StatsCollectorTest.java b/server/src/test/java/com/cloud/server/StatsCollectorTest.java index 9f428d5d8e0c..2b2451c66c7e 100644 --- a/server/src/test/java/com/cloud/server/StatsCollectorTest.java +++ b/server/src/test/java/com/cloud/server/StatsCollectorTest.java @@ -28,7 +28,6 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VmStats; import com.cloud.vm.VmStatsVO; -import com.cloud.vm.dao.VmStatsDao; import com.cloud.vm.dao.VmStatsDaoImpl; import com.google.gson.Gson; import com.tngtech.java.junit.dataprovider.DataProvider;