-
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
Allow counters to be created with same name, provider and source as a deleted one #10223
base: 4.19
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10223 +/- ##
=========================================
Coverage 15.16% 15.16%
- Complexity 11296 11299 +3
=========================================
Files 5408 5408
Lines 473891 473895 +4
Branches 57838 57838
=========================================
+ Hits 71846 71851 +5
Misses 394013 394013
+ Partials 8032 8031 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@blueorangutan package |
@Pearl1594 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 12137 |
@blueorangutan test |
@rohityadavcloud 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.
one comment,
Also the issue reported that deleteCounter doesn't work. Is that genuine?
ALTER TABLE `cloud`.`counter` DROP KEY `uc_counter__provider__source__value`; | ||
CALL `cloud`.`IDEMPOTENT_ADD_UNIQUE_KEY`('cloud.counter', 'uc_counter__provider__source__value_removed', '(provider, source, value, removed)'); |
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.
how will we deal with this in upgrade paths that bypass this one?
I mean can we add an if-exists construct and copy it to future upgrades?
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.
Why would this be bypassed? Wouldn't we have a schema file for 4.19.2 -> 4.20? And since you brought it up, since this involves a DB change - should this go into the next major release?
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.
If I understand correctly, what Daan meant is the upgrade from a version which does not have this change , for example 4.20.0.0 ?
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.
Ah.. got it.. I completely forgot 🤦
[SF] Trillian test result (tid-12142)
|
@@ -22,5 +22,20 @@ | |||
-- Add last_id to the volumes table | |||
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.volumes', 'last_id', 'bigint(20) unsigned DEFAULT NULL'); | |||
|
|||
ALTER TABLE `cloud`.`counter` DROP KEY `uc_counter__provider__source__value`; | |||
CALL `cloud`.`IDEMPOTENT_ADD_UNIQUE_KEY`('cloud.counter', 'uc_counter__provider__source__value_removed', '(provider, source, value, removed)'); | |||
SELECT |
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.
@Pearl1594
maybe we can add a procedure similar as
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.
good call @weizhouapache that will help with a lot of these parallel upgrade issues.
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
engine/schema/src/main/resources/META-INF/db/schema-41910to41920.sql
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
Description
This PR fixes: #10043
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?