Fix resource count discrepancies #8302

vishesh92 · 2023-12-05T09:09:45Z

Description

Needs some rework which will be done after #8362 is merged.

This PR fixes the resource count discrepancies which happen when resource count is being incremented or decremented and recalculation of resource count happens at the same time.

Requires 2 Management servers to reproduce

On MS1, Add a debugger at

cloudstack/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java

Line 889 in 7243946

for (ResourceCountVO rowToUpdate : rowsToUpdate) {
Deploy a VM.
When the debugger stops at above line, execute cmk update resourcecount domainid=`1on MS2 to trigger recalculation of resource count (this also happens periodically. cmk command triggers the same method on demand). cmk command will get blocked because of the debugger.
Resume the debugger.
cmk command will complete and you will see the discrepancy error in logs.

You will see a log line with the following text

Discrepency in the resource count has been detected (original count = 1 correct count = 2) for Type = user_vm for Domain ID = 2 is fixed during resource count recalculation

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

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

vishesh92 · 2023-12-05T09:12:36Z

@blueorangutan package

blueorangutan · 2023-12-05T09:16:03Z

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

codecov · 2023-12-05T09:17:36Z

Codecov Report

Attention: Patch coverage is 75.86207% with 49 lines in your changes are missing coverage. Please review.

Project coverage is 30.92%. Comparing base (6dc3d06) to head (7dcc999).

Files	Patch %	Lines
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java	62.96%	9 Missing and 1 partial ⚠️
.../cloud/resourcelimit/ResourceLimitManagerImpl.java	87.09%	1 Missing and 7 partials ⚠️
...n/java/com/cloud/vm/VirtualMachineManagerImpl.java	61.11%	4 Missing and 3 partials ⚠️
...va/com/cloud/resourcelimit/CheckedReservation.java	73.07%	5 Missing and 2 partials ⚠️
...main/java/com/cloud/storage/dao/VolumeDaoImpl.java	64.70%	4 Missing and 2 partials ⚠️
...cloudstack/reservation/dao/ReservationDaoImpl.java	83.78%	6 Missing ⚠️
.../src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java	72.72%	1 Missing and 2 partials ⚠️
...g/apache/cloudstack/reservation/ReservationVO.java	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #8302       +/-   ##
=============================================
+ Coverage     15.88%   30.92%   +15.04%     
- Complexity    15718    33688    +17970     
=============================================
  Files          5172     5397      +225     
  Lines        364426   379499    +15073     
  Branches      53574    55373     +1799     
=============================================
+ Hits          57874   117354    +59480     
+ Misses       299648   246565    -53083     
- Partials       6904    15580     +8676

Flag	Coverage Δ
simulator-marvin-tests	`24.41% <71.92%> (?)`
uitests	`4.34% <ø> (ø)`
unit-tests	`16.88% <41.87%> (+<0.01%)`	⬆️

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

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

blueorangutan · 2023-12-05T09:40:16Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 7924

vishesh92 · 2023-12-05T12:19:22Z

@blueorangutan package

blueorangutan · 2023-12-05T12:20:03Z

@vishesh92 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 · 2023-12-05T13:49:45Z

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

vishesh92 · 2023-12-06T06:16:10Z

@blueorangutan package

blueorangutan · 2023-12-06T06:28:03Z

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

yadvr · 2023-12-06T07:49:04Z

@vishesh92 does it affect only 4.18, or we're aiming to fix in main/4.19+ ?

vishesh92 · 2023-12-06T08:29:37Z

@vishesh92 does it affect only 4.18, or we're aiming to fix in main/4.19+ ?

@rohityadavcloud Both 4.18 & main are affected. This requires a migration which will make the upgrade path a little complex with 4.18.2. So, I have raised this PR against main/4.19.

vishesh92 · 2023-12-11T03:55:45Z

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

-            _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.cpu, displayVm, cpu);
-            _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.memory, displayVm, memory);
+        if (!VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
+            Transaction.execute(new TransactionCallbackNoReturn() {


Another way to do this without incrementing all three in a transaction would require adding columns for each reservation type (user_vm, cpu, memory) in user_vm table since cpu & memory are calculated using the user_vm table.

Similar solution for primary_storage & volume as well.

vishesh92 · 2023-12-11T04:14:04Z

@blueorangutan package

blueorangutan · 2024-03-05T09:24:03Z

@rohityadavcloud a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

vishesh92 · 2024-03-05T11:42:47Z

@blueorangutan package

blueorangutan · 2024-03-05T11:44:03Z

@vishesh92 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 · 2024-03-05T12:57:28Z

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

blueorangutan · 2024-03-05T22:36:16Z

[SF] Trillian test result (tid-9393)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46035 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9393-xenserver-71.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2024-03-05T22:58:44Z

[SF] Trillian test result (tid-9395)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47264 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9395-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2024-03-06T00:27:26Z

[SF] Trillian test result (tid-9394)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 52646 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9394-vmware-67u3.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_balanced_drs_algorithm	`Error`	435.05	test_cluster_drs.py

vishesh92 · 2024-03-06T08:51:56Z

@blueorangutan test matrix

blueorangutan · 2024-03-06T08:54:03Z

@vishesh92 a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2024-03-06T21:44:45Z

[SF] Trillian test result (tid-9401)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44625 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9401-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2024-03-06T22:23:54Z

[SF] Trillian test result (tid-9399)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47034 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9399-xenserver-71.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2024-03-07T02:51:28Z

[SF] Trillian test result (tid-9400)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 63088 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9400-vmware-67u3.zip
Smoke tests completed. 127 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_balanced_drs_algorithm	`Error`	435.71	test_cluster_drs.py
test_10_reboot_cpvm_forced	`Error`	19.30	test_ssvm.py

github-actions · 2024-03-08T10:40:55Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

yadvr · 2024-03-12T11:40:51Z

@vishesh92 can you address the merge conflict and re-run the packaging and smoktests

…screpancies

vishesh92 · 2024-03-12T15:39:04Z

@blueorangutan package

blueorangutan · 2024-03-12T15:40:04Z

@vishesh92 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 · 2024-03-12T16:51:10Z

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

vishesh92 · 2024-03-12T17:22:49Z

@blueorangutan test

blueorangutan · 2024-03-12T17:24:04Z

@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2024-03-13T07:13:13Z

[SF] Trillian test result (tid-9451)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48193 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8302-t9451-kvm-centos7.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_hostha_enable_ha_when_host_disabled	`Error`	3.79	test_hostha_kvm.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	306.30	test_hostha_kvm.py

* Fix resource count discrepancies * Fixup while removing vm * Fix discrepancies when starting VMs * Fixup tests * Fix failing tests * Don't take lock when amount is negative --------- Co-authored-by: dahn <daan@onecht.net>

vishesh92 changed the base branch from main to 4.18 December 5, 2023 09:10

vishesh92 force-pushed the fix-resource-count-discrepancies branch 2 times, most recently from 45d6668 to e7ec0d4 Compare December 5, 2023 10:17

vishesh92 force-pushed the fix-resource-count-discrepancies branch from e7ec0d4 to f1516aa Compare December 6, 2023 06:14

boring-cyborg bot added component:agent component:api component:build component:integration labels Dec 6, 2023

vishesh92 changed the base branch from 4.18 to main December 6, 2023 06:14

vishesh92 force-pushed the fix-resource-count-discrepancies branch from f1516aa to 61488bc Compare December 6, 2023 06:15

vishesh92 added this to the 4.19.0.0 milestone Dec 6, 2023

vishesh92 force-pushed the fix-resource-count-discrepancies branch from 61488bc to c3812d6 Compare December 11, 2023 03:51

boring-cyborg bot added component:orchestration component:storage labels Dec 11, 2023

vishesh92 commented Dec 11, 2023

View reviewed changes

vishesh92 marked this pull request as ready for review December 11, 2023 04:13

vishesh92 requested review from DaanHoogland and shwstppr December 11, 2023 04:13

Don't take lock when amount is negative

3eb213a

github-actions bot added the status:has-conflicts label Mar 8, 2024

Merge remote-tracking branch 'origin/main' into fix-resource-count-di…

7dcc999

…screpancies

github-actions bot removed the status:has-conflicts label Mar 12, 2024

yadvr marked this pull request as ready for review March 13, 2024 12:52

yadvr merged commit e87c6cf into apache:main Mar 13, 2024

yadvr deleted the fix-resource-count-discrepancies branch March 13, 2024 12:52

GutoVeronezi mentioned this pull request Apr 2, 2024

Add ReservationDao as dependency for the Usage Server #8868

Merged

11 tasks

vishesh92 mentioned this pull request Apr 17, 2024

Wrong resource usage often reported in multiple area #8927

Closed

Fix resource count discrepancies #8302

Fix resource count discrepancies #8302

Uh oh!

Conversation

vishesh92 commented Dec 5, 2023 • 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

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

vishesh92 commented Dec 5, 2023

Uh oh!

blueorangutan commented Dec 5, 2023

Uh oh!

codecov bot commented Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Dec 5, 2023

Uh oh!

vishesh92 commented Dec 5, 2023

Uh oh!

blueorangutan commented Dec 5, 2023

Uh oh!

blueorangutan commented Dec 5, 2023

Uh oh!

vishesh92 commented Dec 6, 2023

Uh oh!

blueorangutan commented Dec 6, 2023

Uh oh!

yadvr commented Dec 6, 2023

Uh oh!

vishesh92 commented Dec 6, 2023

Uh oh!

vishesh92 Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

vishesh92 Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

vishesh92 commented Dec 11, 2023

Uh oh!

blueorangutan commented Mar 5, 2024

Uh oh!

vishesh92 commented Mar 5, 2024

Uh oh!

blueorangutan commented Mar 5, 2024

Uh oh!

blueorangutan commented Mar 5, 2024

Uh oh!

blueorangutan commented Mar 5, 2024

Uh oh!

blueorangutan commented Mar 5, 2024

Uh oh!

blueorangutan commented Mar 6, 2024

Uh oh!

vishesh92 commented Mar 6, 2024

Uh oh!

blueorangutan commented Mar 6, 2024

Uh oh!

blueorangutan commented Mar 6, 2024

Uh oh!

blueorangutan commented Mar 6, 2024

Uh oh!

blueorangutan commented Mar 7, 2024

Uh oh!

github-actions bot commented Mar 8, 2024

Uh oh!

yadvr commented Mar 12, 2024

Uh oh!

vishesh92 commented Mar 12, 2024

Uh oh!

blueorangutan commented Mar 12, 2024

Uh oh!

blueorangutan commented Mar 12, 2024

vishesh92 commented Dec 5, 2023 •

edited

Loading

codecov bot commented Dec 5, 2023 •

edited

Loading