-
Notifications
You must be signed in to change notification settings - Fork 1.2k
CPU to Memory weight based algorithm to order cluster #10997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
host.capacityType.to.order.clusters config will support new algorithm: COMBINED which will work with host.capacityType.to.order.clusters.cputomemoryweight and capacity will be computed based on CPU and memory both and using weight factor
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10997 +/- ##
============================================
+ Coverage 16.56% 16.58% +0.01%
- Complexity 14010 14036 +26
============================================
Files 5758 5758
Lines 511578 511717 +139
Branches 62192 62216 +24
============================================
+ Hits 84756 84870 +114
- Misses 417350 417374 +24
- Partials 9472 9473 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@sudo87 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. |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 13745 |
@blueorangutan package |
@sudo87 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. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13751 |
@sudo87 will you create a doc PR for this as well? |
Yes @DaanHoogland, doc pr will be needed for this change. |
engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@sudo87 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. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13821 |
@blueorangutan test |
@sudo87 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13554)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blueorangutan package
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14022 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for a new "COMBINED" ordering mode that ranks clusters, pods, and hosts by a weighted sum of CPU and memory usage, introduces a config key for the CPU-to-memory weight, updates the DAO and schema to fetch both capacity types, and adds related unit tests.
- Added
COMBINED
option andHostCapacityTypeCpuMemoryWeight
config key - Extended
FirstFitPlanner
,FirstFitAllocator
, andCapacityDao
to compute and sort by combined metrics - Updated tests, configuration enum, and DB migration for the new behavior
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
server/src/main/java/com/cloud/deploy/FirstFitPlanner.java | Added helpers for combined CPU/memory ordering and refactored cluster/pod listing |
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java | Added combined-capacity ordering and helper methods for hosts |
engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java | Introduced HostCapacityTypeCpuMemoryWeight config key |
server/src/main/java/com/cloud/configuration/Config.java | Updated Config.HostCapacityTypeToOrderClusters to include COMBINED |
engine/schema/src/main/resources/META-INF/db/schema-42010to42100.sql | Migrated config description to mention COMBINED |
engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java | Updated DAO interface signatures for capacity listing |
engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java | Added methods to list capacities by types and removed obsolete signatures |
server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java | Added tests for combined ordering on clusters and pods |
server/src/test/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocatorTest.java | Added tests for combined ordering on hosts |
engine/schema/src/test/java/com/cloud/capacity/dao/CapacityDaoImplTest.java | Extended DAO tests for new listing methods |
Comments suppressed due to low confidence (4)
server/src/main/java/com/cloud/deploy/FirstFitPlanner.java:526
- Missing import for ArrayList: add 'import java.util.ArrayList;' to ensure this compiles.
return new Pair<>(new ArrayList<>(podsByCombinedCapacities.keySet()), podsByCombinedCapacities);
server/src/main/java/com/cloud/deploy/FirstFitPlanner.java:564
- Missing import for ArrayList: add 'import java.util.ArrayList;' to the top of the file.
return new Pair<>(new ArrayList<>(clusterByCombinedCapacities.keySet()), clusterByCombinedCapacities);
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java:424
- Missing import for HashMap: add 'import java.util.HashMap;' so this compiles without errors.
Map<Long, Double> hostByComputedCapacity = new HashMap<>();
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java:419
- Missing import for ArrayList: please add 'import java.util.ArrayList;'.
return new Pair<>(new ArrayList<>(hostByComputedCapacity.keySet()), hostByComputedCapacity);
@blueorangutan package |
@sudo87 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. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14116 |
return new Pair<>(new ArrayList<>(podsByCombinedCapacities.keySet()), podsByCombinedCapacities); | ||
} | ||
|
||
// order pods by combining cpu and memory capacity considering cpuToMemoeryWeight |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be javadoc? (it seems the method name already is self-documenting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure @DaanHoogland, will remove comment in next commit.
@blueorangutan test |
@rosi-shapeblue a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-13752)
|
@blueorangutan test |
@sudo87 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Verification passed.
[SF] Trillian test result (tid-13764)
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
* CPU to Memory weight based algorithm to order cluster host.capacityType.to.order.clusters config will support new algorithm: COMBINED which will work with host.capacityType.to.order.clusters.cputomemoryweight and capacity will be computed based on CPU and memory both and using weight factor * minor changes * add unit tests * update desc and add validation * handle copilot review comments * add log indicating chosen capacityType for ordering --------- Co-authored-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Description
This PR introduces new value: "COMBINED" for config: "host.capacityType.to.order.clusters", which will be used to order cluster, host and pods based on CPU and Memory both.
COMBINED will work with "host.capacityType.to.order.clusters.cputomemoryweight" and overall capacity for cluster/pod/host will be computed based on CPU and memory using weight factor.
The allocator will need to first calculate the combined allocation/usage metric (as follows), before sorting the clusters/pods/hosts to return a ordered list of hosts by this metric, for example:
For each host, define metric as:
Doc PR: apache/cloudstack-documentation#524
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?