Alpha162 opened a new pull request, #13933: URL: https://github.com/apache/cloudstack/pull/13933
### Description Backport of #13909 to `4.22`, per @winterhazel's guidance on that PR, targeting 4.22.2 as @abh1sar flagged. The DDL is identical to what merged on main. It also creates the 4.22.1.0 to 4.22.2.0 upgrade path, which didn't exist yet. The chain in `DatabaseUpgradeChecker` ended at `Upgrade42200to42210`, and since the branch is already `4.22.2.0-SNAPSHOT`, 4.22.2.0 is the declared next version. Nothing had needed a schema change on 4.22 since 4.22.1.0 was cut, so nobody had reason to add it. The CALLs can't go into `schema-42200to42210.sql` instead. Every 4.22.1.0 install has already run that file, and those are the affected clusters. The `-cleanup.sql` has no statements in it, but the file has to exist: `getCleanupScripts()` throws if it's absent. See #13909 for the note on NULL semantics in the widened key, and on this preventing the problem rather than remediating clusters that already have it. ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [X] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? Not tested end to end. The environment where this was found has since been rebuilt, so I can't exercise a 4.22.1.0 to 4.22.2.0 upgrade myself. What I can attest to: - The DDL is byte-identical to what merged on main in #13909, which passed packaging on el8/el9/el10/debian/suse15 and a Trillian smoke run there (156 pass, 0 errors, tid-16778). - The narrow `UNIQUE KEY id (volume_id, created)` was confirmed by `SHOW CREATE TABLE` against a restored backup of a live 4.22.1.0 install, alongside the `vm_id` column added by #11531. - The key name `id` is set explicitly by `schema-41600to41610.sql:72`, so the drop targets a name present on any install that has passed through 4.16.1.0. - Widening a unique key is strictly less restrictive, so no existing row can violate the new constraint and the DDL can't fail on data. - Both procedures carry `CONTINUE HANDLER`s (1091 and 1061), so a re-run or a resumed upgrade is a no-op. The new upgrade path is the part that genuinely wants a reviewer with an environment. `Upgrade42210to42220` is the first 4.22.1 to 4.22.2 step on this branch, and its script filenames are derived from `getUpgradableVersionRange()` rather than declared, so the naming is mechanically correct. But nobody has run a real upgrade through it. I'd appreciate someone doing that before merge. #### How did you try to break this feature and the system with this change? Reasoned through the failure modes rather than executing them, since I have no environment to run an upgrade in. - A 4.22.0.x cluster upgrading straight to 4.22.2.0. Traced the chain: `Upgrade42200to42210` runs first and adds `vm_id` with the narrow key, then `Upgrade42210to42220` widens it. Both populations land correct. - A cluster where an operator has already widened the key by hand. `IDEMPOTENT_DROP_INDEX` drops the three-column key and `IDEMPOTENT_ADD_UNIQUE_INDEX` recreates it identically, so the net effect is the same. - A re-run or a resumed upgrade. Handlers 1091 and 1061 swallow the missing-index and duplicate-key-name cases, so both CALLs are no-ops second time round. - A missing or misnamed cleanup script. `getCleanupScripts()` throws `CloudRuntimeException` when the file is absent, so that fails the upgrade rather than silently skipping it. Both script names derive from `getUpgradableVersionRange()`, so they can't drift from the class referencing them. - Existing data violating the widened key. That can't happen; the new key is less restrictive than the one it replaces. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
