Backup usage: bill each size for the time it applied, and publish the metric only on change by abh1sar · Pull Request #14310 · apache/cloudstack

abh1sar · 2026-10-05T11:21:32Z

Description

This PR fixes how VM backup usage is recorded and billed:

Backup usage billed at the last reported size for the whole period (VM Backup usage billed at last-reported size for the entire billing period #13070). A BACKUP.USAGE.METRIC event overwrote the size of the active usage_backup row in place, so BackupUsageParser applied the last size to the entire aggregation window. A size change now closes the active row at the event date and opens a new row with the new size, so each size is billed only for the time it was in effect. An unchanged size is a no-op, rows that are already removed are no longer updated, and duplicate active rows for the same VM and offering are merged into one. A metric for a VM and offering without active usage is still ignored, now with a warning.
Backups billed twice after re-assigning an offering. Removing a backup offering while keeping the backups publishes no usage event, so the usage row stays active (correctly, as the backups still use storage). Re-assigning the same offering then added a second active row, and both were billed. The usage server now skips the new row when one is already active.
BACKUP.USAGE.METRIC published on every backup sync. The backup sync published the metric for every VM on every run (backup.framework.sync.interval, 5 minutes by default) whether or not it changed. The last value published per VM and offering is now kept in a new backup_usage_metric table, and the metric is only published when the current value differs from it. The event and the stored value are committed in the same transaction, so the stored value is always what the usage server last received, whichever management server sent it.
Backup sync running concurrently on every management server. Every management server runs the backup sync for every zone with no coordination. Each zone is now synced under a per-zone global lock (backup.sync.<zoneId>), as BucketUsageTask does for bucket usage. This keeps the usage metric comparison consistent across management servers, and also stops two servers from adding or removing the same out-of-band backups (Veeam, Networker) at the same time, which created duplicate backup rows and double resource count updates.
Usage of the current offering never dropped to zero. The backup sync only reported the VM's current offering when the VM had no backups at all. If the VM still had backups from an earlier offering, deleting the last backup of its current offering left that offering's usage at its last size. The current offering is now always reported.

Behaviour changes:

New table cloud.backup_usage_metric (upgrade from 4.22.1.0 to 4.22.2.0). A row is removed when the offering usage is removed (BACKUP.OFFERING.BACKUPS.DEL).
usage_backup gains a row per size change instead of one row per assignment, and billing records are split accordingly.
BACKUP.USAGE.METRIC is published once per actual change instead of once per VM per sync, so far fewer rows in cloud.usage_event and cloud_usage.usage_event. Existing rows are left in place. Management server restarts do not republish.
On upgrade the table is empty, so the first sync publishes each VM's metric once. This also merges any duplicate active usage_backup rows created by the re-assign bug.
A zone whose sync lock is held by another management server is skipped for that run.

Fixes: #13070

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

Bug Severity

How Has This Been Tested?

Tested on 4.22 with NAS backup on an NFS repository. The usage job ran with a 15 minute aggregation range so each window could be checked.

Before the fix:

The VM's backup size changed at 08:58, but every window from 00:00 was billed at the new size.
The VM had two active usage_backup rows from an earlier remove and re-assign of the offering, so every window had two billing records.
98.8% of all usage events were BACKUP.USAGE.METRIC (73,669 events with 64 distinct values for 2 VMs).

Usage after the fix (VM backup usage records per 15 minute window):

Scenario	Window	Period billed	Size	Duration
Duplicate rows merged at the first metric event	09:26 to 09:41	09:26:00 to 09:38:04	18.81 GB and 22.19 GB (two rows)	12.07 min each
		09:38:04 to 09:40:59	22.19 GB (one row)	2.93 min
Backup taken	06:48 to 07:03	06:48:00 to 07:00:10	18.81 GB	12.17 min
		07:00:10 to 07:02:59	22.19 GB	2.83 min
Backup deleted	07:03 to 07:18	07:03:00 to 07:05:10	22.19 GB	2.17 min
		07:05:10 to 07:17:59	18.81 GB	12.83 min

Before the fix, each of these windows would have been billed at a single size for the full 15 minutes (twice, while the duplicate row existed).

Also:

No BACKUP.USAGE.METRIC events were published for 16 hours overnight while sizes did not change. Before, one was published per VM every 5 minutes.
Removing and re-assigning the offering with backups kept: the re-assign was skipped and one active row remained.

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

Restarted the management server: nothing was republished, as the stored values matched.
Simulated another management server that published a different value and then went down, by inserting its event and stored value directly: the next sync published the real value again and the usage server split the row at that time.
Held the zone sync lock from another database session: the whole zone sync (storage stats, out-of-band backups, usage metrics) was skipped for that run and the next run caught up.
Created and deleted backups within one aggregation window: each change got its own row and billing record.
A republished unchanged value was ignored by the usage server.
Metric for a VM without an active usage row: ignored, as before.

codecov · 2026-10-05T11:31:20Z

Codecov Report

❌ Patch coverage is 63.70370% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.04%. Comparing base (2e63c60) to head (8086034).
⚠️ Report is 6 commits behind head on 4.22.

Files with missing lines	Patch %	Lines
.../apache/cloudstack/backup/BackupUsageMetricVO.java	48.64%	19 Missing ⚠️
...loudstack/backup/dao/BackupUsageMetricDaoImpl.java	0.00%	19 Missing ⚠️
...n/java/com/cloud/usage/dao/UsageBackupDaoImpl.java	72.72%	8 Missing and 1 partial ⚠️
...rg/apache/cloudstack/backup/BackupManagerImpl.java	95.12%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #14310      +/-   ##
============================================
+ Coverage     18.00%   18.04%   +0.04%     
- Complexity    16219    16273      +54     
============================================
  Files          5936     5938       +2     
  Lines        535716   535890     +174     
  Branches      65596    65615      +19     
============================================
+ Hits          96459    96706     +247     
+ Misses       428268   428182      -86     
- Partials      10989    11002      +13

Flag	Coverage Δ
uitests	`4.04% <ø> (+0.02%)`	⬆️
unittests	`19.12% <63.70%> (+0.04%)`	⬆️

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.

Copilot

Copilot review overview

🔵 Needs a closer look

Stateful billing chronology and in-memory event deduplication warrant final human validation.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes backup usage accounting so size changes are billed proportionally while reducing redundant metric events.

Changes:

Splits active usage rows when backup sizes change and merges duplicates.
Prevents duplicate rows when offerings are reassigned.
Publishes metrics only when values change and reports current offerings consistently.

File	Description
`usage/src/test/java/com/cloud/usage/UsageManagerImplTest.java`	Tests assignment and metric handling.
`usage/src/main/java/com/cloud/usage/UsageManagerImpl.java`	Prevents duplicate active usage and forwards event timestamps.
`server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java`	Tests metric deduplication and zero-size reporting.
`server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java`	Caches published metrics and reports current offerings.
`engine/schema/src/test/java/com/cloud/usage/dao/UsageBackupDaoImplTest.java`	Tests usage-row splitting and duplicate consolidation.
`engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java`	Implements timestamped metric history and active-row merging.
`engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDao.java`	Extends the backup usage DAO contract.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Damans227 · 2026-10-05T12:07:37Z

+                    Pair<Long, Long> key = new Pair<>(vm.getId(), offeringId);
+                    Pair<Long, Long> metric = new Pair<>(backupSize, protectedSize);
+                    published.put(key, metric);
+                    if (metric.equals(lastPublished.get(key))) {


what happens with two management servers if one sends a new size and then goes down? if the size goes back after that, the other one still remembers the old value and skips it, so billing stays on the wrong size

Thanks @Damans227 . That's a big issue with the current implementation.
I'll think about an alternative.

Added a new BackupUsageMetrics table in DB to track the last updated value.
Also added a lock so that two management servers don't race while syncing the backup metrics.
Please take a look @Damans227

the new table and the lock cover it, thanks

A BACKUP.USAGE.METRIC event overwrote the size of the usage_backup row in place, so the parser billed the last reported size for the whole period. A size change now closes the active row at the event date and opens a new one. Unchanged sizes are ignored since the metric can repeat an unchanged value, closed rows are no longer updated, and duplicate active rows for the same VM and offering are merged. Fixes apache#13070.

Removing a backup offering while keeping the backups leaves its usage row active. Re-assigning the same offering added another active row, so the same backups were billed twice. Skip the new row when one is active.

The backup sync published a BACKUP.USAGE.METRIC event for every VM on every run (every 5 minutes by default) even when nothing changed, which made up most of the usage_event rows. Keep the last value published per VM and offering in the new backup_usage_metric table and skip unchanged ones. The event and the stored value are committed together. Every management server runs the backup sync, so each zone is now synced under a per-zone lock. The usage metrics are then always compared against the last published values, and two servers no longer add or remove the same out-of-band backups at the same time. The VM's current offering is now always reported. It was skipped when the VM still had backups from an earlier offering, so deleting its last backup never brought its usage down to zero.

abh1sar · 2026-10-06T08:18:51Z

@blueorangutan package

Copilot

Copilot review overview

🟡 Changes recommended

Metric-state deletion is not atomic with its usage event, allowing stale values to suppress later billing metrics.

Review effort: Balanced
Findings: 1

Open (1)

Wrap deletion event and metric removal in one transaction · New

Publish the usage event for an offering whose backups are all gone and delete its row in backup_usage_metric in one transaction, as is already done when publishing the metric, so a stale stored value cannot outlive the usage.

Damans227 · 2026-10-06T11:00:09Z

+            txn.commit();
        } catch (final Exception e) {
+            txn.rollback();
            logger.error("Error updating backup metrics: " + e.getMessage(), e);


what happens if this fails once? the size is already saved as sent, so it never gets sent again and billing stays on the old size until it changes

Damans227 · 2026-10-06T11:25:55Z

 --;
+
+-- Last backup usage metric published per VM and backup offering
+CREATE TABLE IF NOT EXISTS `cloud`.`backup_usage_metric` (


can we hook the 4.22.2 upgrade step into the upgrade checker here? right now this table never gets made and the backup smoke test fails deleting a backup

boring-cyborg Bot added component:backup component:usage-server labels Oct 5, 2026

abh1sar marked this pull request as draft October 5, 2026 11:24

abh1sar force-pushed the backup-usage-history branch from ec6e3ef to 5dbc2ef Compare October 5, 2026 11:45

abh1sar marked this pull request as ready for review October 5, 2026 11:56

abh1sar requested a balanced review from Copilot October 5, 2026 11:56

Copilot started reviewing on behalf of abh1sar October 5, 2026 11:59 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Damans227 reviewed Oct 5, 2026

View reviewed changes

abh1sar marked this pull request as draft October 5, 2026 13:49

abh1sar added 3 commits October 6, 2026 11:13

Usage: do not add a second backup usage row on offering re-assign

5a64aad

Removing a backup offering while keeping the backups leaves its usage row active. Re-assigning the same offering added another active row, so the same backups were billed twice. Skip the new row when one is active.

abh1sar force-pushed the backup-usage-history branch from 5dbc2ef to 48169b4 Compare October 6, 2026 07:37

abh1sar requested review from Damans227 and a balanced review from Copilot October 6, 2026 08:18

Copilot started reviewing on behalf of abh1sar October 6, 2026 08:19 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Outdated

abh1sar force-pushed the backup-usage-history branch from 6df3ef2 to 8086034 Compare October 6, 2026 08:38

abh1sar marked this pull request as ready for review October 6, 2026 08:38

abh1sar linked an issue Oct 6, 2026 that may be closed by this pull request

VM Backup usage billed at last-reported size for the entire billing period #13070

Open

abh1sar added this to the 4.22.2 milestone Oct 6, 2026

Damans227 reviewed Oct 6, 2026

View reviewed changes

Navigation Menu

Backup usage: bill each size for the time it applied, and publish the metric only on change - #14310

Backup usage: bill each size for the time it applied, and publish the metric only on change#14310
abh1sar wants to merge 4 commits into
apache:4.22from
shapeblue:backup-usage-history

abh1sar commented Oct 5, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Damans227 Oct 5, 2026

Uh oh!

abh1sar Oct 5, 2026

Uh oh!

abh1sar Oct 6, 2026

Uh oh!

Damans227 Oct 6, 2026

Uh oh!

abh1sar commented Oct 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Damans227 Oct 6, 2026

Uh oh!

Damans227 Oct 6, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

abh1sar commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Bug Severity

How Has This Been Tested?

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

Uh oh!

codecov Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Damans227 Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

abh1sar Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

abh1sar Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

abh1sar commented Oct 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Damans227 Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abh1sar commented Oct 5, 2026 •

edited

Loading

codecov Bot commented Oct 5, 2026 •

edited

Loading