Skip to content

fix: correct VM snapshot usage attribution (two defects) - #13924

Open
waterWang wants to merge 1 commit into
apache:mainfrom
waterWang:fix-vm-snapshot-usage-attribution
Open

fix: correct VM snapshot usage attribution (two defects)#13924
waterWang wants to merge 1 commit into
apache:mainfrom
waterWang:fix-vm-snapshot-usage-attribution

Conversation

@waterWang

Copy link
Copy Markdown

Fixes #13921

Two defects corrected in VMSnapshotUsageParser:

  1. Wrong snapshot ID label in usage record: createUsageRecord() used the current snapshot's ID (usageRec.getVmSnapshotId()) while duration, size, and disk offering came from the previous event. Changed to previousEvent.getVmSnapshotId().

  2. Concurrent snapshot key collision: The unprocessedUsage map keyed entries by vmId + ":" + volId only, so concurrent snapshots of the same volume overwrote each other. Added vmSnapshotId to the key to distinguish them.

Root cause

From the issue body:

Net effect: the first snapshot's usage is credited to the second snapshot, and the first snapshot stops accruing usage even though it still exists and is Ready.

@boring-cyborg

boring-cyborg Bot commented Aug 20, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

Fixes apache#13921

Defect 1: createUsageRecord used the current snapshot ID (usageRec)
for the usage label, while duration/size/diskOffering came from the
previous event. This caused the first snapshot's usage to be credited
to the second snapshot.

Defect 2: unprocessedUsage key was vmId + volId only, so concurrent
snapshots of the same volume overwrote each other. Added vmSnapshotId
to the key to distinguish them.

Signed-off-by: waterWang <waterwang@proton.me>
@waterWang
waterWang force-pushed the fix-vm-snapshot-usage-attribution branch from 3857686 to 6cf39e1 Compare August 20, 2026 05:14
@harikrishna-patnala

Copy link
Copy Markdown
Member

thanks for the fix @waterWang, can you please change the branch 4.22 ?

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.65%. Comparing base (d45d481) to head (6cf39e1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../com/cloud/usage/parser/VMSnapshotUsageParser.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13924      +/-   ##
============================================
- Coverage     19.65%   19.65%   -0.01%     
+ Complexity    19801    19796       -5     
============================================
  Files          6368     6368              
  Lines        574913   574913              
  Branches      70352    70352              
============================================
- Hits         113024   112999      -25     
- Misses       449616   449643      +27     
+ Partials      12273    12271       -2     
Flag Coverage Δ
uitests 3.41% <ø> (ø)
unittests 20.93% <0.00%> (-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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VM snapshot usage records are attributed to the wrong snapshot; older snapshots stop accruing usage entirely

3 participants