Skip to content

Add configuration to limit the number of rows deleted from vm_stats#8740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 5 commits into from
Jun 20, 2024
Merged

Add configuration to limit the number of rows deleted from vm_stats #8740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a2f238b
Add configuration to limit the number of rows deleted from vm_stats
Feb 23, 2024
ef39209
Address review
JoaoJandre May 31, 2024
81bb609
fix tests
JoaoJandre Jun 12, 2024
c47ddc6
fix checkstyle
JoaoJandre Jun 12, 2024
d401488
Merge remote-tracking branch 'origin/4.19' into add-limit-to-vm_stats…
JoaoJandre Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,8 +75,10 @@ public interface VmStatsDao extends GenericDao<VmStatsVO, Long> {
/**
* 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.
* If 0 or negative, no limit is used.
*/
void removeAllByTimestampLessThan(Date limit);
void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery);

}
21 changes: 18 additions & 3 deletions engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,6 +21,7 @@

import javax.annotation.PostConstruct;

import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;

import com.cloud.utils.db.Filter;
Expand All@@ -33,6 +34,8 @@
@Component
public class VmStatsDaoImpl extends GenericDaoBase<VmStatsVO, Long> implements VmStatsDao {

protected Logger logger = Logger.getLogger(getClass());

protected SearchBuilder<VmStatsVO> vmIdSearch;
protected SearchBuilder<VmStatsVO> vmIdTimestampGreaterThanEqualSearch;
protected SearchBuilder<VmStatsVO> vmIdTimestampLessThanEqualSearch;
Expand DownExpand Up@@ -113,10 +116,22 @@ public void removeAllByVmId(long vmId) {
}

@Override
public void removeAllByTimestampLessThan(Date limit) {
public void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery) {
SearchCriteria<VmStatsVO> 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));
}

}
8 changes: 8 additions & 0 deletions framework/db/src/main/java/com/cloud/utils/db/GenericDao.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -229,6 +229,14 @@ public interface GenericDao<T, ID extends Serializable> {
*/
int expunge(final SearchCriteria<T> 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<T> sc, long limit);

/**
* expunge the removed rows.
*/
Expand Down
12 changes: 11 additions & 1 deletion framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1226,9 +1226,14 @@ public boolean expunge(final ID id) {
}
}

// FIXME: Does not work for joins.
@Override
public int expunge(final SearchCriteria<T> sc) {
return expunge(sc, -1);
}

// FIXME: Does not work for joins.
@Override
public int expunge(final SearchCriteria<T> sc, long limit) {
Comment thread
JoaoJandre marked this conversation as resolved.
if (sc == null) {
throw new CloudRuntimeException("Call to throw new expunge with null search Criteria");
}
Expand All@@ -1241,6 +1246,11 @@ public int expunge(final SearchCriteria<T> sc) {
str.append(sc.getWhereClause());
}

if (limit > 0) {
Comment thread
JoaoJandre marked this conversation as resolved.
str.append(" LIMIT ");
str.append(limit);
}

final String sql = str.toString();

final TransactionLegacy txn = TransactionLegacy.currentTxn();
Expand Down
12 changes: 7 additions & 5 deletions server/src/main/java/com/cloud/server/StatsCollector.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -287,6 +284,11 @@ 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" +
Comment thread
JoaoJandre marked this conversation as resolved.
" 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);

Expand DownExpand Up@@ -1952,7 +1954,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());
}

/**
Expand DownExpand Up@@ -2126,7 +2128,7 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri,
return new ConfigKey<?>[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, vmStatsRemoveBatchSize,
vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime,
MANAGEMENT_SERVER_STATUS_COLLECTION_INTERVAL,
DATABASE_SERVER_STATUS_COLLECTION_INTERVAL,
Expand Down
8 changes: 4 additions & 4 deletions server/src/test/java/com/cloud/server/StatsCollectorTest.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,7 +28,7 @@
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;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
Expand DownExpand Up@@ -81,7 +81,7 @@ public class StatsCollectorTest {
private static final String DEFAULT_DATABASE_NAME = "cloudstack";

@Mock
VmStatsDao vmStatsDaoMock = Mockito.mock(VmStatsDao.class);
VmStatsDaoImpl vmStatsDaoMock;

@Mock
VmStatsEntry statsForCurrentIterationMock;
Expand DownExpand Up@@ -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.anyLong());
}

@Test
Expand All@@ -313,7 +313,7 @@ public void cleanUpVirtualMachineStatsTestIsEnabled() {

statsCollector.cleanUpVirtualMachineStats();

Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any());
Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any(), Mockito.anyLong());
}

@Test
Expand Down
5 changes: 5 additions & 0 deletions server/src/test/java/com/cloud/user/MockUsageEventDao.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -210,6 +210,11 @@ public int expunge(SearchCriteria<UsageEventVO> sc) {
return 0;
}

@Override
public int expunge(SearchCriteria<UsageEventVO> sc, long limit) {
return 0;
}

@Override
public void expunge() {

Expand Down