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());
+        }
+    }
+
+}

Reply via email to