-
Notifications
You must be signed in to change notification settings - Fork 1.3k
server: improve storage GC to skip expunging possible duplicate volumes #7313
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
server: improve storage GC to skip expunging possible duplicate volumes #7313
Conversation
627eedc to
c85e114
Compare
|
@blueorangutan package |
|
@shwstppr a 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: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5683 |
|
@blueorangutan test matrix |
|
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
Trillian test result (tid-6262)
|
|
Trillian test result (tid-6263)
|
|
Trillian test result (tid-6264)
|
rohityadavcloud
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.
Didn't test this, LGTM - this would need testing (one concern that can be tested if, this doesn't introduce any regression in the GC logic such that valid volumes that must be GC'd and deleted are no longer deleted)
|
Changing base branch creates conflicts @rohityadavcloud @DaanHoogland |
…icate Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
c85e114 to
8a0564d
Compare
|
@blueorangutan package |
|
@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. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6121 |
|
@blueorangutan test |
|
@rohityadavcloud a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian Build Failed (tid-6589) |
|
@blueorangutan LLtest |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[LL]Trillian test result (tid-6544)
|
|
@blueorangutan test |
|
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-6594) |
|
@blueorangutan LLtest |
|
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[LL]Trillian test result (tid-6549)
|
|
@blueorangutan test matrix |
|
@DaanHoogland a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
@blueorangutan package |
|
@shwstppr a [LL] 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 [LL]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6067 |
|
@blueorangutan test matrix |
|
@DaanHoogland a [SF] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-6664)
|
|
[SF] Trillian test result (tid-6665)
|
|
[SF] Trillian test result (tid-6666)
|
|
This seems ready to merge, I am just missing some validation and given the description it seems hard to reproduce/test. |
|
This is just a defensive check and I don't think it would be simple to reproduce. At most, we can test that there is no regression in the GC logic. I've also tried to add multiple unit tests to prevent any unnecessary change in the behaviour |
Thanks, @shwstppr I think we did enough for regression testing, looking at the code change (scope), merging |
Description
Storage GC deletes volumes that are in Destroy state. This PR adds a check in this action that is if two volume records (one in Destroy state and one in Ready state) point to exactly the same disk file then skip doing GC on such Destroy state volume record.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?