This is an automated email from the ASF dual-hosted git repository.
bstoyanov pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 7d52cd0e43a Fix calculation of the next time that Usage will execute
in `removeRawUsageRecords` (#12518)
7d52cd0e43a is described below
commit 7d52cd0e43a5e225f10bce251f7cd357060382cd
Author: Fabricio Duarte <[email protected]>
AuthorDate: Thu Jan 29 10:38:12 2026 -0300
Fix calculation of the next time that Usage will execute in
`removeRawUsageRecords` (#12518)
* Fix calculation of the next time that Usage will execute in
`removeRawUsageRecords`
* Address copilot reviews
---
.../java/com/cloud/usage/UsageServiceImpl.java | 60 ++++-----
.../java/com/cloud/usage/UsageManagerImpl.java | 35 ++----
.../apache/cloudstack/utils/usage/UsageUtils.java | 51 ++++++++
.../cloudstack/utils/usage/UsageUtilsTest.java | 135 +++++++++++++++++++++
4 files changed, 230 insertions(+), 51 deletions(-)
diff --git a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java
b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java
index edaa22c3bcf..de8d4633d22 100644
--- a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java
+++ b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java
@@ -17,7 +17,6 @@
package com.cloud.usage;
import java.util.ArrayList;
-import java.util.Calendar;
import java.util.Date;
import java.util.List;
import java.util.Map;
@@ -35,6 +34,7 @@ import
org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.usage.Usage;
import org.apache.cloudstack.usage.UsageService;
import org.apache.cloudstack.usage.UsageTypes;
+import org.apache.cloudstack.utils.usage.UsageUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
@@ -127,14 +127,25 @@ public class UsageServiceImpl extends ManagerBase
implements UsageService, Manag
@Inject
private NetworkOfferingDao _networkOfferingDao;
+ private TimeZone usageExecutionTimeZone = TimeZone.getTimeZone("GMT");
+
+ private static final long REMOVE_RAW_USAGE_RECORDS_WINDOW_IN_MS = 15 * 60
* 1000;
+
public UsageServiceImpl() {
}
@Override
public boolean configure(String name, Map<String, Object> params) throws
ConfigurationException {
super.configure(name, params);
+
String timeZoneStr =
ObjectUtils.defaultIfNull(_configDao.getValue(Config.UsageAggregationTimezone.toString()),
"GMT");
_usageTimezone = TimeZone.getTimeZone(timeZoneStr);
+
+ String executionTimeZone =
_configDao.getValue(Config.UsageExecutionTimezone.toString());
+ if (executionTimeZone != null) {
+ usageExecutionTimeZone = TimeZone.getTimeZone(executionTimeZone);
+ }
+
return true;
}
@@ -465,35 +476,28 @@ public class UsageServiceImpl extends ManagerBase
implements UsageService, Manag
@Override
public boolean removeRawUsageRecords(RemoveRawUsageRecordsCmd cmd) throws
InvalidParameterValueException {
Integer interval = cmd.getInterval();
- if (interval != null && interval > 0 ) {
- String jobExecTime =
_configDao.getValue(Config.UsageStatsJobExecTime.toString());
- if (jobExecTime != null ) {
- String[] segments = jobExecTime.split(":");
- if (segments.length == 2) {
- String timeZoneStr =
_configDao.getValue(Config.UsageExecutionTimezone.toString());
- if (timeZoneStr == null) {
- timeZoneStr = "GMT";
- }
- TimeZone tz = TimeZone.getTimeZone(timeZoneStr);
- Calendar cal = Calendar.getInstance(tz);
- cal.setTime(new Date());
- long curTS = cal.getTimeInMillis();
- cal.set(Calendar.HOUR_OF_DAY,
Integer.parseInt(segments[0]));
- cal.set(Calendar.MINUTE, Integer.parseInt(segments[1]));
- cal.set(Calendar.SECOND, 0);
- cal.set(Calendar.MILLISECOND, 0);
- long execTS = cal.getTimeInMillis();
- logger.debug("Trying to remove old raw cloud_usage records
older than " + interval + " day(s), current time=" + curTS + " next job
execution time=" + execTS);
- // Let's avoid cleanup when job runs and around a 15 min
interval
- if (Math.abs(curTS - execTS) < 15 * 60 * 1000) {
- return false;
- }
- }
+ if (interval == null || interval <= 0) {
+ throw new InvalidParameterValueException("Interval should be
greater than 0.");
+ }
+
+ String jobExecTime =
_configDao.getValue(Config.UsageStatsJobExecTime.toString());
+ Date previousJobExecTime =
UsageUtils.getPreviousJobExecutionTime(usageExecutionTimeZone, jobExecTime);
+ Date nextJobExecTime =
UsageUtils.getNextJobExecutionTime(usageExecutionTimeZone, jobExecTime);
+ if (ObjectUtils.allNotNull(previousJobExecTime, nextJobExecTime)) {
+ logger.debug("Next Usage job is scheduled to execute at [{}];
previous execution was at [{}].",
+ DateUtil.displayDateInTimezone(usageExecutionTimeZone,
nextJobExecTime), DateUtil.displayDateInTimezone(usageExecutionTimeZone,
previousJobExecTime));
+ Date now = new Date();
+ if (nextJobExecTime.getTime() - now.getTime() <
REMOVE_RAW_USAGE_RECORDS_WINDOW_IN_MS) {
+ logger.info("Not removing any cloud_usage records because the
next Usage job is scheduled to execute in less than {} minute(s).",
REMOVE_RAW_USAGE_RECORDS_WINDOW_IN_MS / 60000);
+ return false;
+ } else if (now.getTime() - previousJobExecTime.getTime() <
REMOVE_RAW_USAGE_RECORDS_WINDOW_IN_MS) {
+ logger.info("Not removing any cloud_usage records because the
last Usage job executed in less than {} minute(s) ago.",
REMOVE_RAW_USAGE_RECORDS_WINDOW_IN_MS / 60000);
+ return false;
}
- _usageDao.expungeAllOlderThan(interval,
ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
- } else {
- throw new InvalidParameterValueException("Invalid interval value.
Interval to remove cloud_usage records should be greater than 0");
}
+
+ logger.info("Removing cloud_usage records older than {} day(s).",
interval);
+ _usageDao.expungeAllOlderThan(interval,
ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
return true;
}
}
diff --git a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
index 30cdfcf21f0..9da64889fc3 100644
--- a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
+++ b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java
@@ -198,7 +198,9 @@ public class UsageManagerImpl extends ManagerBase
implements UsageManager, Runna
private Future _heartbeat = null;
private Future _sanity = null;
private boolean usageSnapshotSelection = false;
+
private static TimeZone usageAggregationTimeZone =
TimeZone.getTimeZone("GMT");
+ private static TimeZone usageExecutionTimeZone =
TimeZone.getTimeZone("GMT");
public UsageManagerImpl() {
}
@@ -253,6 +255,9 @@ public class UsageManagerImpl extends ManagerBase
implements UsageManager, Runna
if (aggregationTimeZone != null && !aggregationTimeZone.isEmpty()) {
usageAggregationTimeZone =
TimeZone.getTimeZone(aggregationTimeZone);
}
+ if (execTimeZone != null) {
+ usageExecutionTimeZone = TimeZone.getTimeZone(execTimeZone);
+ }
try {
if ((execTime == null) || (aggregationRange == null)) {
@@ -261,34 +266,18 @@ public class UsageManagerImpl extends ManagerBase
implements UsageManager, Runna
throw new ConfigurationException("Missing configuration values
for usage job, usage.stats.job.exec.time = " + execTime +
", usage.stats.job.aggregation.range = " +
aggregationRange);
}
- String[] execTimeSegments = execTime.split(":");
- if (execTimeSegments.length != 2) {
- logger.error("Unable to parse usage.stats.job.exec.time");
- throw new ConfigurationException("Unable to parse
usage.stats.job.exec.time '" + execTime + "'");
- }
- int hourOfDay = Integer.parseInt(execTimeSegments[0]);
- int minutes = Integer.parseInt(execTimeSegments[1]);
-
- Date currentDate = new Date();
- _jobExecTime.setTime(currentDate);
-
- _jobExecTime.set(Calendar.HOUR_OF_DAY, hourOfDay);
- _jobExecTime.set(Calendar.MINUTE, minutes);
- _jobExecTime.set(Calendar.SECOND, 0);
- _jobExecTime.set(Calendar.MILLISECOND, 0);
-
- TimeZone jobExecTimeZone = execTimeZone != null ?
TimeZone.getTimeZone(execTimeZone) : Calendar.getInstance().getTimeZone();
- _jobExecTime.setTimeZone(jobExecTimeZone);
- // if the hour to execute the job has already passed, roll the day
forward to the next day
- if (_jobExecTime.getTime().before(currentDate)) {
- _jobExecTime.roll(Calendar.DAY_OF_YEAR, true);
+ Date nextJobExecTime =
UsageUtils.getNextJobExecutionTime(usageExecutionTimeZone, execTime);
+ if (nextJobExecTime == null) {
+ throw new ConfigurationException(String.format("Unable to
parse configuration 'usage.stats.job.exec.time' value [%s].", execTime));
}
+ _jobExecTime.setTimeZone(usageExecutionTimeZone);
+ _jobExecTime.setTime(nextJobExecTime);
logger.info("Usage is configured to execute in time zone [{}], at
[{}], each [{}] minutes; the current time in that timezone is [{}] and the " +
"next job is scheduled to execute at [{}]. During
its execution, Usage will aggregate stats according to the time zone [{}]
defined in global setting [usage.aggregation.timezone].",
- jobExecTimeZone.getID(), execTime, aggregationRange,
DateUtil.displayDateInTimezone(jobExecTimeZone, currentDate),
- DateUtil.displayDateInTimezone(jobExecTimeZone,
_jobExecTime.getTime()), usageAggregationTimeZone.getID());
+ usageExecutionTimeZone.getID(), execTime,
aggregationRange, DateUtil.displayDateInTimezone(usageExecutionTimeZone, new
Date()),
+ DateUtil.displayDateInTimezone(usageExecutionTimeZone,
_jobExecTime.getTime()), usageAggregationTimeZone.getID());
_aggregationDuration = Integer.parseInt(aggregationRange);
if (_aggregationDuration < UsageUtils.USAGE_AGGREGATION_RANGE_MIN)
{
diff --git
a/utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
b/utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
index a97aed15d36..861788d1918 100644
--- a/utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
+++ b/utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
@@ -19,6 +19,57 @@
package org.apache.cloudstack.utils.usage;
+import com.cloud.utils.DateUtil;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.util.Calendar;
+import java.util.Date;
+import java.util.TimeZone;
+
public class UsageUtils {
+ protected static Logger logger = LogManager.getLogger(UsageUtils.class);
+
public static final int USAGE_AGGREGATION_RANGE_MIN = 1;
+
+ public static Date getNextJobExecutionTime(TimeZone usageTimeZone, String
jobExecTimeConfig) {
+ return getJobExecutionTime(usageTimeZone, jobExecTimeConfig, true);
+ }
+
+ public static Date getPreviousJobExecutionTime(TimeZone usageTimeZone,
String jobExecTimeConfig) {
+ return getJobExecutionTime(usageTimeZone, jobExecTimeConfig, false);
+ }
+
+ protected static Date getJobExecutionTime(TimeZone usageTimeZone, String
jobExecTimeConfig, boolean next) {
+ String[] execTimeSegments = jobExecTimeConfig.split(":");
+ if (execTimeSegments.length != 2) {
+ logger.warn("Unable to parse configuration
'usage.stats.job.exec.time'.");
+ return null;
+ }
+ int hourOfDay;
+ int minutes;
+ try {
+ hourOfDay = Integer.parseInt(execTimeSegments[0]);
+ minutes = Integer.parseInt(execTimeSegments[1]);
+ } catch (NumberFormatException e) {
+ logger.warn("Unable to parse configuration
'usage.stats.job.exec.time' due to non-numeric values in [{}].",
jobExecTimeConfig, e);
+ return null;
+ }
+
+ Date currentDate = DateUtil.currentGMTTime();
+ Calendar jobExecTime = Calendar.getInstance(usageTimeZone);
+ jobExecTime.setTime(currentDate);
+ jobExecTime.set(Calendar.HOUR_OF_DAY, hourOfDay);
+ jobExecTime.set(Calendar.MINUTE, minutes);
+ jobExecTime.set(Calendar.SECOND, 0);
+ jobExecTime.set(Calendar.MILLISECOND, 0);
+
+ if (next && jobExecTime.getTime().before(currentDate)) {
+ jobExecTime.add(Calendar.DAY_OF_YEAR, 1);
+ } else if (!next && jobExecTime.getTime().after(currentDate)) {
+ jobExecTime.add(Calendar.DAY_OF_YEAR, -1);
+ }
+
+ return jobExecTime.getTime();
+ }
}
diff --git
a/utils/src/test/java/org/apache/cloudstack/utils/usage/UsageUtilsTest.java
b/utils/src/test/java/org/apache/cloudstack/utils/usage/UsageUtilsTest.java
new file mode 100644
index 00000000000..8b9b4910e39
--- /dev/null
+++ b/utils/src/test/java/org/apache/cloudstack/utils/usage/UsageUtilsTest.java
@@ -0,0 +1,135 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package org.apache.cloudstack.utils.usage;
+
+import com.cloud.utils.DateUtil;
+import junit.framework.TestCase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Date;
+import java.util.TimeZone;
+
+@RunWith(MockitoJUnitRunner.class)
+public class UsageUtilsTest extends TestCase {
+
+ TimeZone usageTimeZone = TimeZone.getTimeZone("GMT-3");
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsNullWhenConfigurationValueIsInvalid() {
+ Date result = UsageUtils.getNextJobExecutionTime(usageTimeZone,
"test");
+ assertNull(result);
+ }
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsExpectedDateWhenNextIsTrueAndExecutionTimeHasNotPassed()
{
+ Date currentDate = new Date();
+ currentDate.setTime(1724296800000L);
+
+ try (MockedStatic<DateUtil> dateUtilMockedStatic =
Mockito.mockStatic(DateUtil.class)) {
+
dateUtilMockedStatic.when(DateUtil::currentGMTTime).thenReturn(currentDate);
+
+ Date result = UsageUtils.getJobExecutionTime(usageTimeZone,
"00:30", true);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(1724297400000L, result.getTime());
+ }
+ }
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsExpectedDateWhenNextIsTrueAndExecutionTimeHasPassed()
{
+ Date currentDate = new Date();
+ currentDate.setTime(1724297460000L);
+
+ try (MockedStatic<DateUtil> dateUtilMockedStatic =
Mockito.mockStatic(DateUtil.class)) {
+
dateUtilMockedStatic.when(DateUtil::currentGMTTime).thenReturn(currentDate);
+
+ Date result = UsageUtils.getJobExecutionTime(usageTimeZone,
"00:30", true);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(1724383800000L, result.getTime());
+ }
+ }
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsExpectedDateWhenNextIsFalseAndExecutionTimeHasNotPassed()
{
+ Date currentDate = new Date();
+ currentDate.setTime(1724296800000L);
+
+ try (MockedStatic<DateUtil> dateUtilMockedStatic =
Mockito.mockStatic(DateUtil.class)) {
+
dateUtilMockedStatic.when(DateUtil::currentGMTTime).thenReturn(currentDate);
+
+ Date result = UsageUtils.getJobExecutionTime(usageTimeZone,
"00:30", false);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(1724211000000L, result.getTime());
+ }
+ }
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsExpectedDateWhenNextIsFalseAndExecutionTimeHasPassed()
{
+ Date currentDate = new Date();
+ currentDate.setTime(1724297460000L);
+
+ try (MockedStatic<DateUtil> dateUtilMockedStatic =
Mockito.mockStatic(DateUtil.class)) {
+
dateUtilMockedStatic.when(DateUtil::currentGMTTime).thenReturn(currentDate);
+
+ Date result = UsageUtils.getJobExecutionTime(usageTimeZone,
"00:30", false);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(1724297400000L, result.getTime());
+ }
+ }
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsExpectedDateWhenNextExecutionIsOnNextYear() {
+ Date currentDate = new Date();
+ currentDate.setTime(1767236340000L);
+
+ try (MockedStatic<DateUtil> dateUtilMockedStatic =
Mockito.mockStatic(DateUtil.class)) {
+
dateUtilMockedStatic.when(DateUtil::currentGMTTime).thenReturn(currentDate);
+
+ Date result = UsageUtils.getJobExecutionTime(usageTimeZone,
"00:00", true);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(1767236400000L, result.getTime());
+ }
+ }
+
+ @Test
+ public void
getJobExecutionTimeTestReturnsExpectedDateWhenPreviousExecutionWasOnPreviousYear()
{
+ Date currentDate = new Date();
+ currentDate.setTime(1767236460000L);
+
+ try (MockedStatic<DateUtil> dateUtilMockedStatic =
Mockito.mockStatic(DateUtil.class)) {
+
dateUtilMockedStatic.when(DateUtil::currentGMTTime).thenReturn(currentDate);
+
+ Date result = UsageUtils.getJobExecutionTime(usageTimeZone,
"23:59", false);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(1767236340000L, result.getTime());
+ }
+ }
+
+}