-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix ordering of secondary storages with the algorithm firstfitleastconsumed
#8557
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
Fix ordering of secondary storages with the algorithm firstfitleastconsumed
#8557
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8557 +/- ##
============================================
+ Coverage 16.07% 16.92% +0.85%
- Complexity 12885 12893 +8
============================================
Files 5642 5248 -394
Lines 494039 461620 -32419
Branches 59912 54182 -5730
============================================
- Hits 79408 78135 -1273
+ Misses 405828 374830 -30998
+ Partials 8803 8655 -148
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:
|
...src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java
Outdated
Show resolved
Hide resolved
|
@blueorangutan package |
|
@sureshanaparti 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8428 |
DaanHoogland
left a comment
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.
clgtm
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
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.
CLGTM
|
@blueorangutan package |
|
@GaOrtiga 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8833 |
|
@sureshanaparti is this ok now? do we need more testing? |
|
@blueorangutan package |
|
@JoaoJandre 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9278 |
|
@DaanHoogland @sureshanaparti @rohityadavcloud @shwstppr could we run the CI here? |
|
ping @sureshanaparti |
|
@sureshanaparti could you trigger the CI here? |
sorry @BryanMLima , our lab is a bit tired at the moment. I first wanted to hear from @sureshanaparti if he is alright with the current state of the PR |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-10013)
|
|
@DaanHoogland @sureshanaparti, could you trigger the CI here? |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@BryanMLima @GaOrtiga pls resolve the conflicts. thanks. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11147 |
|
@blueorangutan LLtest |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
…rages_with_firstfitleastconsumed
|
@blueorangutan package |
|
@winterhazel 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 11974 |
winterhazel
left a comment
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.
Looks good, did some tests manually in an environment with two secondary storages (one with 26 GB available and another with 50 GB available) using firstfitleastconsumed. Before the changes, the storage with 26 GB would be listed first. After the patch, the 50 GB storage gets listed first.
|
@DaanHoogland could we run the CI to proceed with the merge? |
sureshanaparti
left a comment
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.
clgtm
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-12035)
|
|
@DaanHoogland test failures seem unrelated and also failed in other PRs, could we rerun or proceed with the merge? |
|
@blueorangutan package |
|
@DaanHoogland 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. |
@winterhazel , the kubernetes failures were an issue on main that should be fixed. I'm doing an extra round just to be sure about the pvlan error. I don't think it is related though. ok? |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12089 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-12111) |
|
[SF] Trillian Build Failed (tid-12112) |
|
[LL]Trillian test result (tid-7055)
|
…onsumed` (apache#8557) * Fix ordering of secondary storages with the algorithm `firstfitleastconsumed` * return store without checking all * Add unit tests --------- Co-authored-by: Gabriel <[email protected]> Co-authored-by: Fabricio Duarte <[email protected]>
The algorithm
firstfitleastconsumedorders the secondary storages based on how much free capacity each has and returns the first item of the list that has enough free capacity. However, the ordering of this list is inverted, putting the storages with the least free capacity at the top.This behaviour has been adjusted, altering the order of the list to guarantee that the secondary storages with the most free capacity get allocated first.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity