server: clear resource reservation and increment resource count in a transaction by shwstppr · Pull Request #7724 · apache/cloudstack

shwstppr · 2023-07-06T03:57:42Z

Description

This PR addresses rare cases of potential overlap of resource reservation and resource count. For different resource types there could be some delay between incrementing of the resource count and clearing of the earlier done reservation. This may result in failures when there are parallel deployments happening.

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

…transaction This PR addresses rare case of potential overlap of resource reservation and resource count. For different resource types there could be some delay between incrementing of the resource count and clearing of the earlier done reservation. This may result in failures when there are parallel deployments happening. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2023-07-06T04:03:58Z

@blueorangutan package

blueorangutan · 2023-07-06T04:06:03Z

@shwstppr a [SF] 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-07-06T04:53:47Z

Codecov Report

Merging #7724 (05e57f6) into 4.18 (9e1609d) will increase coverage by 0.00%.
The diff coverage is 3.81%.

@@            Coverage Diff             @@
##               4.18    #7724    +/-   ##
==========================================
  Coverage     13.01%   13.01%            
- Complexity     9011     9026    +15     
==========================================
  Files          2716     2719     +3     
  Lines        256656   256877   +221     
  Branches      40021    40052    +31     
==========================================
+ Hits          33410    33445    +35     
- Misses       219060   219243   +183     
- Partials       4186     4189     +3

Impacted Files	Coverage Δ
.../src/main/java/com/cloud/host/dao/HostDaoImpl.java	`27.85% <0.00%> (-0.59%)`	⬇️
...main/java/com/cloud/storage/dao/VolumeDaoImpl.java	`23.23% <0.00%> (-0.39%)`	⬇️
...en/api/command/ConfigTungstenFabricServiceCmd.java	`0.00% <ø> (ø)`
...api/storage/StorPoolSetVolumeEncryptionAnswer.java	`0.00% <0.00%> (ø)`
...pi/storage/StorPoolSetVolumeEncryptionCommand.java	`0.00% <0.00%> (ø)`
...per/StorPoolSetVolumeEncryptionCommandWrapper.java	`0.00% <0.00%> (ø)`
...tastore/driver/StorPoolPrimaryDataStoreDriver.java	`0.00% <0.00%> (ø)`
...ack/storage/motion/StorPoolDataMotionStrategy.java	`0.00% <0.00%> (ø)`
...oud/network/element/ConfigDriveNetworkElement.java	`26.79% <0.00%> (-0.26%)`	⬇️
...n/java/com/cloud/resource/ResourceManagerImpl.java	`0.00% <0.00%> (ø)`
... and 3 more

... and 17 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

blueorangutan · 2023-07-06T04:55:47Z

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

DaanHoogland

code looks good but can you put the transaction in a separate method and not in with the precondition checks, please.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2023-07-06T08:43:21Z

@DaanHoogland refactored

@blueorangutan package

blueorangutan · 2023-07-06T08:44:03Z

@shwstppr a [SF] 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.

harikrishna-patnala

I hit this issue too. Thanks @shwstppr
Code LGTM

blueorangutan · 2023-07-06T09:36:43Z

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

DaanHoogland · 2023-07-06T09:50:05Z

@blueorangutan test

blueorangutan · 2023-07-06T09:54:03Z

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

shwstppr · 2023-07-09T15:28:39Z

@blueorangutan test

blueorangutan · 2023-07-09T15:34:03Z

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

blueorangutan · 2023-07-10T06:08:12Z

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

Test	Result	Time (s)	Test File
test_01_migrate_VM_and_root_volume	`Error`	77.77	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	56.53	test_vm_life_cycle.py
test_01_vpc_site2site_vpn_multiple_options	`Error`	694.22	test_vpc_vpn.py

kiranchavala

LGTM , Manually tested the fix

Before fix

Logs messages

"errortext":"Maximum domain resource limits of Type 'user_vm' for Domain Id = 435 is exceeded: Domain Resource Limit = 5, Current Domain Resource Amount = 4, Current Resource Reservation = 1, Requested Resource Amount = 1."

Updated the domain limit to 5, and deployed 6 vm in parallel

2023-07-18 16:26:49,994 ERROR [c.c.r.ResourceLimitManagerImpl] (qtp737826424-1595:ctx-66f84de9 ctx-be6fe214 ctx-d8954cd3) (logid:91ec9d9f) Maximum domain resource limits of Type 'user_vm' for Domain Id = 2 is exceeded: Domain Resource Limit = (5 bytes) 5, Current Domain Resource Amount = (6 bytes) 6, Requested Resource Amount = (1 bytes) 1.
com.cloud.exception.ResourceAllocationException: Maximum domain resource limits of Type 'user_vm' for Domain Id = 2 is exceeded: Domain Resource Limit = (5 bytes) 5, Current Domain Resource Amount = (6 bytes) 6, Requested Resource Amount = (1 bytes) 1.
com.cloud.exception.ResourceAllocationException: Maximum domain resource limits of Type 'user_vm' for Domain Id = 2 is exceeded: Domain Resource Limit = (5 bytes) 5, Current Domain Resource Amount = (6 bytes) 6, Requested Resource Amount = (1 bytes) 1.

After fix

Logs messages

2023-07-18 17:32:05,911 ERROR [c.c.r.ResourceLimitManagerImpl] (qtp1008561904-26:ctx-aaceba8c ctx-1561cceb ctx-b957dd77) (logid:8e101cc4) Maximum domain resource limits of Type 'user_vm' for Domain Id = 2 is exceeded: Domain Resource Limit = 5, Current Domain Resource Amount = 0, Current Resource Reservation = 5, Requested Resource Amount = 1.
com.cloud.exception.ResourceAllocationException: Maximum domain resource limits of Type 'user_vm' for Domain Id = 2 is exceeded: Domain Resource Limit = 5, Current Domain Resource Amount = 0, Current Resource Reservation = 5, Requested Resource Amount = 1.
com.cloud.exception.ResourceAllocationException: Maximum domain resource limits of Type 'user_vm' for Domain Id = 2 is exceeded: Domain Resource Limit = 5, Current Domain Resource Amount = 0, Current Resource Reservation = 5, Requested Resource Amount = 1.

DaanHoogland · 2023-07-27T10:17:54Z

@blueorangutan package

blueorangutan · 2023-07-27T10:20:03Z

@DaanHoogland a [SF] 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-07-27T11:31:39Z

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

boring-cyborg Bot added the component:api label Jul 6, 2023

DaanHoogland approved these changes Jul 6, 2023

View reviewed changes

refactor

05e57f6

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

harikrishna-patnala approved these changes Jul 6, 2023

View reviewed changes

kiranchavala approved these changes Jul 20, 2023

View reviewed changes

yadvr added this to the 4.18.1.0 milestone Jul 21, 2023

yadvr marked this pull request as ready for review July 21, 2023 05:25

yadvr merged commit cee7a71 into apache:4.18 Jul 21, 2023

yadvr deleted the fix-reservation-resourcecount-overlap branch July 21, 2023 05:25

Conversation

shwstppr commented Jul 6, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

shwstppr commented Jul 6, 2023

Uh oh!

blueorangutan commented Jul 6, 2023

Uh oh!

codecov Bot commented Jul 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Jul 6, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr commented Jul 6, 2023

Uh oh!

blueorangutan commented Jul 6, 2023

Uh oh!

harikrishna-patnala left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jul 6, 2023

Uh oh!

DaanHoogland commented Jul 6, 2023

Uh oh!

blueorangutan commented Jul 6, 2023

Uh oh!

shwstppr commented Jul 9, 2023

Uh oh!

blueorangutan commented Jul 9, 2023

Uh oh!

blueorangutan commented Jul 10, 2023

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jul 27, 2023

Uh oh!

blueorangutan commented Jul 27, 2023

Uh oh!

blueorangutan commented Jul 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov Bot commented Jul 6, 2023 •

edited

Loading