Fix calculation of the next time that Usage will execute in removeRawUsageRecords
#12518
Conversation
|
@blueorangutan package |
|
@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12518 +/- ##
==========================================
Coverage 16.24% 16.25%
- Complexity 13396 13407 +11
==========================================
Files 5658 5658
Lines 499169 499297 +128
Branches 60579 60602 +23
==========================================
+ Hits 81087 81142 +55
- Misses 409041 409108 +67
- Partials 9041 9047 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 16534 |
|
@blueorangutan package |
|
@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16535 |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix a bug in removeRawUsageRecords where the calculation of the next usage job execution time was incorrect when the scheduled time had already passed during the same day. The fix extracts the date calculation logic into a new utility class UsageUtils and uses it consistently across UsageManagerImpl and UsageServiceImpl.
Changes:
- Introduced new utility methods
getNextJobExecutionTimeandgetPreviousJobExecutionTimeinUsageUtilsclass to centralize job execution time calculations - Refactored
UsageManagerImpl.configure()to use the new utility methods instead of inline date calculation - Refactored
UsageServiceImpl.removeRawUsageRecords()to use the new utility methods and improved the logic to check both next and previous job execution times - Added comprehensive unit tests for the new utility methods
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java | New utility class providing centralized methods for calculating next and previous job execution times |
| utils/src/test/java/org/apache/cloudstack/utils/usage/UsageUtilsTest.java | New test file with unit tests for the utility methods |
| usage/src/main/java/com/cloud/usage/UsageManagerImpl.java | Refactored to use new utility methods, removing duplicate date calculation logic |
| server/src/main/java/com/cloud/usage/UsageServiceImpl.java | Refactored removeRawUsageRecords to use new utility methods with improved 15-minute window checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
Outdated
Show resolved
Hide resolved
utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
Outdated
Show resolved
Hide resolved
utils/src/main/java/org/apache/cloudstack/utils/usage/UsageUtils.java
Outdated
Show resolved
Hide resolved
utils/src/test/java/org/apache/cloudstack/utils/usage/UsageUtilsTest.java
Show resolved
Hide resolved
|
@blueorangutan package |
|
@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16565 |
|
[SF] Trillian test result (tid-15294)
|
borisstoyanov
left a comment
borisstoyanov
left a comment
There was a problem hiding this comment.
LGTM, based on code review and Marvin tests. For the actual bug described, your tests are more than sufficient @winterhazel, good work.
5 participants
Description
removeRawUsageRecordscalculates the next moment in which Usage will execute in order to prevent the removal of usage records in a window of 15 minutes before and after the Usage job executes. However, this calculation is performed incorrectly because the date is not rolled to the next day in case the initial calculated date has already passed. Due to this, if the Usage job is scheduled to execute at 00:00 and the current time is 23:59 (the job will execute in a minute), for instance, this check fails and the usage records are removed.This PR fixes the calculation.
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
The tests were performed in a VM with its date set to 2026-01-26 GMT-3.
Before the changes
I scheduled Usage to execute at 00:00 and set the Management Server's date to 23:50. I executed
removeRawUsageRecords. I verified that the calculated execution date was 2026-01-26 00:00 (incorrect value), and that the usage records were removed.With the patch
Regarding
removeRawUsageRecords:I scheduled Usage to execute at 00:00 and set the Management Server's date to 23:50. I executed
removeRawUsageRecords. I verified that the calculated execution date was correct, and that the usage records were not removed.I scheduled Usage to execute at 00:00 and set the Management Server's date to 00:05. I executed
removeRawUsageRecords. I verified that the calculated execution date was correct, and that the usage records were not removed.I scheduled Usage to execute at 00:00 and set the Management Server's date to 23:40. I executed
removeRawUsageRecords. I verified that the calculated execution date was correct, and that the usage records were removed.I scheduled Usage to execute at 00:00 and set the Management Server's date to 00:20. I executed
removeRawUsageRecords. I verified that the calculated execution date was correct, and that the usage records were removed.Regarding the job execution time in the Usage Server itself:
I scheduled Usage to execute at 00:00 and set the Management Server's date to 23:50. I restarted the Usage Server. I verified that the calculated execution date was correct.
I scheduled Usage to execute at 00:00 and set the Management Server's date to 00:20. I restarted the Usage Server. I verified that the calculated execution date was correct.
I scheduled Usage to execute at 00:10 and set the Management Server's date to 00:05. I restarted the Usage Server. I verified that the calculated execution date was correct.
I scheduled Usage to execute at 00:10 and set the Management Server's date to 00:12. I restarted the Usage Server. I verified that the calculated execution date was correct.