-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Enable over provisioning for SharedMountPoint primary storages #8481
Enable over provisioning for SharedMountPoint primary storages #8481
Conversation
@blueorangutan package |
@GutoVeronezi 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.18 #8481 +/- ##
=============================================
+ Coverage 13.12% 29.40% +16.27%
- Complexity 9142 35417 +26275
=============================================
Files 2720 5348 +2628
Lines 257744 414514 +156770
Branches 40182 70287 +30105
=============================================
+ Hits 33839 121883 +88044
- Misses 219615 276663 +57048
- Partials 4290 15968 +11678
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8257 |
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.
code changes lgtm, needs testing with SharedMountPoint pools.
code looks good. I have a worry though. overprovisioning on storage in general is a dangerous thing. On cpu and memory any errors due to overprovisioning are likely to not be fatal, but on storage these can easily lead to data loss. when storage is allocated but not used we can apply overprovisioning but when parts of it get used these should not be considered in overprovisioning. I don't think our model supports such behaviour. any ideas @GutoVeronezi @sureshanaparti (sorry, i don't know what wisdom is in respect to this) |
@GutoVeronezi could you target 4.18.2? |
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, didn't test it
@DaanHoogland |
8981243
to
a7179f0
Compare
@JoaoJandre, done.
@DaanHoogland The configuration |
@blueorangutan package |
@GutoVeronezi 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. |
Yes @GutoVeronezi, there are global settings to (1) notify users, or (2) disable new volume allocation, if allocated capacity or actual used capacity reaches a threshold. The thresholds are around 75% or 85%, so users have enough time to get more storage or clean the storage. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8280 |
ok, I thought only overprovisioned threshold would be signalled. I don't know of an extra guard on the actual threshold , but then again we would expect operators to do their due diligence. Sorry for the over-paranoia. |
no worries @DaanHoogland
|
Aside from the third-party testing, do we still need anything else on this one? |
I don´t know, I didn't test it @GutoVeronezi |
@GutoVeronezi @slavkap |
sorry, I didn't read very well and answered hastely. Yes @GutoVeronezi, clgtm |
77a21bf
to
a7179f0
Compare
@blueorangutan package |
@GutoVeronezi 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 8591 |
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.
@weizhouapache, sorry for the late response!
Unfortunately, I don't have an environment with shared mount point but the code LGTM
thanks @slavkap |
…e#8481) * Enable over provisioning for SharedMountPoint primary storages * Fix unit tests * Fix typos and small adjusts --------- Co-authored-by: Daniel Augusto Veronezi Salvador <gutoveronezi@apache.org>
Description
ACS does not allow operators to set the configuration
storage.overprovisioning.factor
forSharedMountPoint
primary storages, presenting the following message when one tries to change it:Also, the global setting does not impact
SharedMoundPoint
primary storages, which means that the over-provisioning is not applied properly for this storage type.Furthermore, the metric presents that the
SharedMountPoint
has the global over-provisioning applied; however, in a scenario where the over-provision factor is 2, the allocation threshold is 80%, and the metrics show that storage has 40% allocated, one cannot allocate to it anymore.This PR intends to enable over-provisioning for
SharedMountPoint
primary storages.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
In an environment where the global over-provision factor was 1, the allocation threshold was 80%, and the
SharedMountPoint
primary storage had 80% allocated, I was able to change the over-provisioning factor of theSharedMountPoint
primary storage and allocate a new volume on it, as, with the over-provisioning, ACS considered it as having 40% allocated.