DaanHoogland commented on code in PR #13659:
URL: https://github.com/apache/cloudstack/pull/13659#discussion_r3645230145
##########
server/src/main/java/com/cloud/server/StatsCollector.java:
##########
@@ -1286,13 +1286,21 @@ protected Point createInfluxDbPoint(Object
metricsObject) {
*/
class VmStatsCleaner extends ManagedContextRunnable{
protected void runInContext() {
- cleanUpVirtualMachineStats();
+ try {
+ cleanUpVirtualMachineStats();
+ } catch (RuntimeException e) {
+ logger.error("Error trying to clean up VM stats", e);
+ }
}
}
class VolumeStatsCleaner extends ManagedContextRunnable{
protected void runInContext() {
- cleanUpVolumeStats();
+ try {
+ cleanUpVolumeStats();
Review Comment:
```suggestion
@Override
cleanUpVolumeStats();
```
##########
server/src/test/java/com/cloud/server/StatsCollectorTest.java:
##########
@@ -335,6 +335,49 @@ public void cleanUpVirtualMachineStatsTestIsEnabled() {
Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any(),
Mockito.anyLong());
}
+ private void setVmDiskStatsMaxRetentionTimeValue(String value) {
+ StatsCollector.vmDiskStatsMaxRetentionTime = new
ConfigKey<Integer>("Advanced", Integer.class,
"vm.disk.stats.max.retention.time", value,
+ "The maximum time (in minutes) for keeping Volume stats
records in the database. The Volume stats cleanup process will be disabled if
this is set to 0 or less than 0.", true);
+ }
+
+ @Test
+ public void cleanUpVolumeStatsTestIsDisabled() {
+ setVmDiskStatsMaxRetentionTimeValue("0");
+
+ statsCollector.cleanUpVolumeStats();
+
+ Mockito.verify(volumeStatsDao,
Mockito.never()).removeAllByTimestampLessThan(Mockito.any(), Mockito.anyLong());
+ }
+
+ @Test
+ public void cleanUpVolumeStatsTestIsEnabled() {
+ setVmDiskStatsMaxRetentionTimeValue("1");
+
+ statsCollector.cleanUpVolumeStats();
+
+
Mockito.verify(volumeStatsDao).removeAllByTimestampLessThan(Mockito.any(),
Mockito.anyLong());
+ }
+
+ @Test
+ public void
vmStatsCleanerTestCatchesCloudRuntimeExceptionAndKeepsRunning() {
Review Comment:
```suggestion
public void vmStatsCleanerTestCatchesRuntimeExceptionAndKeepsRunning() {
```
##########
server/src/main/java/com/cloud/server/StatsCollector.java:
##########
@@ -1286,13 +1286,21 @@ protected Point createInfluxDbPoint(Object
metricsObject) {
*/
class VmStatsCleaner extends ManagedContextRunnable{
protected void runInContext() {
- cleanUpVirtualMachineStats();
+ try {
+ cleanUpVirtualMachineStats();
+ } catch (RuntimeException e) {
+ logger.error("Error trying to clean up VM stats", e);
+ }
}
}
class VolumeStatsCleaner extends ManagedContextRunnable{
protected void runInContext() {
Review Comment:
```suggestion
@Override
protected void runInContext() {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
