guard against stopping stats background threads

[DaanHoogland]

Description

This PR...

Fixes: #13648

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 16.27%. Comparing base (e8df87e) to head (bd8af1c).
⚠️ Report is 1 commits behind head on 4.20.

Additional details and impacted files

@@            Coverage Diff            @@
##               4.20   #13659   +/-   ##
=========================================
  Coverage     16.26%   16.27%           
- Complexity    13435    13437    +2     
=========================================
  Files          5667     5667           
  Lines        500731   500739    +8     
  Branches      60803    60803           
=========================================
+ Hits          81430    81478   +48     
+ Misses       410197   410155   -42     
- Partials       9104     9106    +2     

Flag	Coverage Δ
uitests	4.14% <ø> (ø)
unittests	17.12% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vishesh92]

clgtm. didn't test

[Copilot]

Pull request overview

This PR aims to prevent the periodic VM/Volume stats cleanup background tasks in StatsCollector from being permanently stopped when a cleanup run throws an exception (per #13648).

Changes:

• Wrapped VmStatsCleaner and VolumeStatsCleaner cleanup calls in try/catch blocks to avoid uncaught exceptions escaping the scheduled task.
• Added unit tests for volume stats cleanup enable/disable behavior and for cleaner exception handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
server/src/main/java/com/cloud/server/StatsCollector.java	Adds exception handling around scheduled cleanup runnables to prevent task termination.
server/src/test/java/com/cloud/server/StatsCollectorTest.java	Adds tests covering volume stats cleanup gating and cleaner exception scenarios.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

server/src/main/java/com/cloud/server/StatsCollector.java:1299

• Add @Override for runInContext() to match the other ManagedContextRunnable inner classes in this file and avoid accidentally changing the method signature without compiler errors.

    class VolumeStatsCleaner extends ManagedContextRunnable{
        protected void runInContext() {
            try {

server/src/test/java/com/cloud/server/StatsCollectorTest.java:341

• The test helper recreates vmDiskStatsMaxRetentionTime with a description that refers to "Volume stats"; the production ConfigKey description is for "VM disks stats". Keeping the text consistent reduces confusion when reading test failures or generated config docs.

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

server/src/test/java/com/cloud/server/StatsCollectorTest.java:373

• Same as the VM cleaner test: the name suggests only CloudRuntimeException is handled and that the task "keeps running", but the code catches RuntimeException and the test asserts non-propagation for a single run(). Rename to match the behavior under test.

    @Test
    public void volumeStatsCleanerTestCatchesCloudRuntimeExceptionAndKeepsRunning() {
        Mockito.doThrow(new CloudRuntimeException("Communications link failure")).when(statsCollector).cleanUpVolumeStats();

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
75.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland 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.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18666

[GaOrtiga]

@DaanHoogland why does it say that you modified every single line from statsCollector.java in your last commit? Am I missing something or is it a glitch?

[DaanHoogland]

@DaanHoogland why does it say that you modified every single line from statsCollector.java in your last commit? Am I missing something or is it a glitch?

It is a glitch, I saw co-pilot’s comment having the same and (apart from not want exactly the same as the bot) tried to do it myself. It looked good before applying the suggestion. This strange view only appeared after I applied my version of the change.

original suggestion: #13659 (comment)
my suggestion: #13659 (comment)

I blame github ;)