fix(ai): add DB migration for per-vendor GPU quota#3353
Open
zstack-robot-1 wants to merge 2 commits into5.5.6from
Open
fix(ai): add DB migration for per-vendor GPU quota#3353zstack-robot-1 wants to merge 2 commits into5.5.6from
zstack-robot-1 wants to merge 2 commits into5.5.6from
Conversation
ZSTAC-82069 Delete orphan GpuDeviceSpecVO records where the parent PciDeviceSpecVO type is not a GPU device type. This cleans up stale data caused by device type changes from GPU to non-GPU during ZSTAC-81489 GPU detection refactoring. GpuDeviceSpecVO uses @PrimaryKeyJoinColumn inheritance from PciDeviceSpecVO. Only records with GPU device types should exist in the child table. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrate existing container.gpu.video.ram.size QuotaVO rows to container.gpu.video.ram.size.nvidia using INSERT IGNORE to support backward-compatible per-vendor GPU quota rollout. ZSTAC-73546 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
概览本次变更在数据库升级脚本中添加了一个新的存储过程，用于将通用的 GPU 配额记录迁移为 NVIDIA 厂商特定的配额记录，通过遍历现有配额条目并插入对应的厂商标记版本来实现数据模式转换。 变更
代码审查工作量评估🎯 2 (Simple) | ⏱️ ~12 分钟 诗
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@conf/db/upgrade/V5.5.6__schema.sql`:
- Around line 184-192: The DELETE should wrap identifiers in backticks and
explicitly include NULL 'type' rows; update the statement targeting
GpuDeviceSpecVO and PciDeviceSpecVO (references to `GpuDeviceSpecVO`,
`PciDeviceSpecVO`, `p`.`type`, `g`.`uuid`, `p`.`uuid`) to use backticks for all
table and column names and change the predicate to treat NULL as non-GPU (e.g.
replace the NOT IN(...) check with a condition that also matches `p`.`type` IS
NULL so rows where `p`.`type` is NULL are deleted).
- Around line 205-228: Wrap all table and column identifiers in the migration
procedure with backticks to avoid reserved-word conflicts: update the DECLARE
cur CURSOR FOR SELECT to use `QuotaVO` and backticked columns (`identityUuid`,
`identityType`, `value`, `name`), update the WHERE clause to `WHERE \`name\` =
'container.gpu.video.ram.size'`, and change the INSERT IGNORE INTO to `INSERT
IGNORE INTO \`QuotaVO\` (\`uuid\`, \`name\`, \`identityUuid\`, \`identityType\`,
\`value\`, \`lastOpDate\`, \`createDate\`) VALUES (...)` (also backtick the
literal column names such as `container.gpu.video.ram.size.nvidia` where used);
ensure every occurrence of QuotaVO and its columns in this procedure (cursor,
FETCH, INSERT) is backticked.
- Around line 199-235: Add idempotency and guard against re-creation: drop the
procedure before creating it, wrap identifiers with backticks, and change the
INSERT logic to only insert when the (name, identityUuid, identityType) tuple
does not already exist. Specifically, add DROP PROCEDURE IF EXISTS
MigrateGpuQuotaPerVendor; before CREATE PROCEDURE MigrateGpuQuotaPerVendor(),
use backticks for `QuotaVO`, `uuid`, `name`, `identityUuid`, `identityType`,
`value`, `lastOpDate`, `createDate`, and replace the INSERT IGNORE INTO QuotaVO
... VALUES(...) with an INSERT INTO `QuotaVO`
(`uuid`,`name`,`identityUuid`,`identityType`,`value`,`lastOpDate`,`createDate`)
SELECT REPLACE(UUID(),'-',''), 'container.gpu.video.ram.size.nvidia',
v_identity_uuid, v_identity_type, v_value, NOW(), NOW() FROM DUAL WHERE NOT
EXISTS (SELECT 1 FROM `QuotaVO` q WHERE
q.`name`='container.gpu.video.ram.size.nvidia' AND
q.`identityUuid`=v_identity_uuid AND q.`identityType`=v_identity_type); keep the
rest of the cursor logic and the final DROP PROCEDURE IF EXISTS call.
Comment on lines
+184
to
+192
| DELETE g FROM GpuDeviceSpecVO g | ||
| INNER JOIN PciDeviceSpecVO p ON g.uuid = p.uuid | ||
| WHERE p.type NOT IN ( | ||
| 'GPU_Video_Controller', | ||
| 'GPU_3D_Controller', | ||
| 'GPU_Processing_Accelerators', | ||
| 'GPU_Co_Processor', | ||
| 'GPU_Communication_Controller' | ||
| ); |
There was a problem hiding this comment.
DELETE 语句需要反引号并补齐 NULL 类型的清理
当前语句未对表/列名加反引号，且 NOT IN 不匹配 NULL，会导致 type 为 NULL 的非 GPU 行未被清理。建议补齐反引号并显式处理 NULL。
🛠️ 建议修改
-DELETE g FROM GpuDeviceSpecVO g
-INNER JOIN PciDeviceSpecVO p ON g.uuid = p.uuid
-WHERE p.type NOT IN (
+DELETE g FROM `GpuDeviceSpecVO` g
+INNER JOIN `PciDeviceSpecVO` p ON g.`uuid` = p.`uuid`
+WHERE (p.`type` NOT IN (
'GPU_Video_Controller',
'GPU_3D_Controller',
'GPU_Processing_Accelerators',
'GPU_Co_Processor',
'GPU_Communication_Controller'
-);
+) OR p.`type` IS NULL);As per coding guidelines, 所有表名和列名必须使用反引号包裹（例如：WHERE system = 1），以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误
🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 184 - 192, The DELETE should
wrap identifiers in backticks and explicitly include NULL 'type' rows; update
the statement targeting GpuDeviceSpecVO and PciDeviceSpecVO (references to
`GpuDeviceSpecVO`, `PciDeviceSpecVO`, `p`.`type`, `g`.`uuid`, `p`.`uuid`) to use
backticks for all table and column names and change the predicate to treat NULL
as non-GPU (e.g. replace the NOT IN(...) check with a condition that also
matches `p`.`type` IS NULL so rows where `p`.`type` is NULL are deleted).
Comment on lines
+199
to
+235
| CREATE PROCEDURE MigrateGpuQuotaPerVendor() | ||
| BEGIN | ||
| DECLARE done INT DEFAULT FALSE; | ||
| DECLARE v_identity_uuid VARCHAR(32); | ||
| DECLARE v_identity_type VARCHAR(255); | ||
| DECLARE v_value BIGINT; | ||
| DECLARE cur CURSOR FOR | ||
| SELECT identityUuid, identityType, value | ||
| FROM QuotaVO | ||
| WHERE name = 'container.gpu.video.ram.size'; | ||
| DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE; | ||
|
|
||
| OPEN cur; | ||
| read_loop: LOOP | ||
| FETCH cur INTO v_identity_uuid, v_identity_type, v_value; | ||
| IF done THEN | ||
| LEAVE read_loop; | ||
| END IF; | ||
|
|
||
| INSERT IGNORE INTO QuotaVO (uuid, name, identityUuid, identityType, value, lastOpDate, createDate) | ||
| VALUES ( | ||
| REPLACE(UUID(), '-', ''), | ||
| 'container.gpu.video.ram.size.nvidia', | ||
| v_identity_uuid, | ||
| v_identity_type, | ||
| v_value, | ||
| NOW(), | ||
| NOW() | ||
| ); | ||
| END LOOP; | ||
| CLOSE cur; | ||
| END$$ | ||
|
|
||
| DELIMITER ; | ||
|
|
||
| CALL MigrateGpuQuotaPerVendor(); | ||
| DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find QuotaVO table definition and constraints
rg -n 'CREATE TABLE.*QuotaVO|CREATE TABLE.*`QuotaVO`' -A 20 -g '*.sql'Repository: MatheMatrix/zstack
Length of output: 1660
🏁 Script executed:
#!/bin/bash
# Search for any UNIQUE or KEY constraints related to QuotaVO
rg -n 'QuotaVO' -B 5 -A 10 -g '*.sql' | rg -A 10 'PRIMARY KEY|UNIQUE|KEY'Repository: MatheMatrix/zstack
Length of output: 5723
🏁 Script executed:
#!/bin/bash
# Check the specific upgrade file for context
find . -name 'V5.5.6__schema.sql' -type f | head -5Repository: MatheMatrix/zstack
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Look for MigrateGpuQuotaPerVendor procedure in the upgrade file
rg -n 'MigrateGpuQuotaPerVendor' -B 5 -A 40 conf/db/upgrade/V5.5.6__schema.sqlRepository: MatheMatrix/zstack
Length of output: 1557
存储过程缺少前置删除检查，且缺乏幂等性保证
CREATE PROCEDURE 未在前置执行 DROP IF EXISTS，若升级中断后重跑会导致执行失败。同时 INSERT IGNORE 因 uuid 采用自动生成且约束在整行上，无法确保业务逻辑的幂等性。当重复执行时，相同的 name、identityUuid 和 identityType 组合会被重复插入。
此外，表名和列名未使用反引号包裹，违反编码规范。
建议修改
+DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor;
DELIMITER $$
CREATE PROCEDURE MigrateGpuQuotaPerVendor()
BEGIN
- DECLARE cur CURSOR FOR
- SELECT identityUuid, identityType, value
- FROM QuotaVO
- WHERE name = 'container.gpu.video.ram.size';
+ DECLARE cur CURSOR FOR
+ SELECT identityUuid, identityType, value
+ FROM `QuotaVO`
+ WHERE `name` = 'container.gpu.video.ram.size';
DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE;
OPEN cur;
read_loop: LOOP
FETCH cur INTO v_identity_uuid, v_identity_type, v_value;
IF done THEN
LEAVE read_loop;
END IF;
- INSERT IGNORE INTO QuotaVO (uuid, name, identityUuid, identityType, value, lastOpDate, createDate)
- VALUES (
- REPLACE(UUID(), '-', ''),
- 'container.gpu.video.ram.size.nvidia',
- v_identity_uuid,
- v_identity_type,
- v_value,
- NOW(),
- NOW()
- );
+ INSERT INTO `QuotaVO` (`uuid`, `name`, `identityUuid`, `identityType`, `value`, `lastOpDate`, `createDate`)
+ SELECT
+ REPLACE(UUID(), '-', ''),
+ 'container.gpu.video.ram.size.nvidia',
+ v_identity_uuid,
+ v_identity_type,
+ v_value,
+ NOW(),
+ NOW()
+ FROM DUAL
+ WHERE NOT EXISTS (
+ SELECT 1
+ FROM `QuotaVO`
+ WHERE `name` = 'container.gpu.video.ram.size.nvidia'
+ AND `identityUuid` = v_identity_uuid
+ AND `identityType` = v_identity_type
+ );🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 199 - 235, Add idempotency
and guard against re-creation: drop the procedure before creating it, wrap
identifiers with backticks, and change the INSERT logic to only insert when the
(name, identityUuid, identityType) tuple does not already exist. Specifically,
add DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor; before CREATE PROCEDURE
MigrateGpuQuotaPerVendor(), use backticks for `QuotaVO`, `uuid`, `name`,
`identityUuid`, `identityType`, `value`, `lastOpDate`, `createDate`, and replace
the INSERT IGNORE INTO QuotaVO ... VALUES(...) with an INSERT INTO `QuotaVO`
(`uuid`,`name`,`identityUuid`,`identityType`,`value`,`lastOpDate`,`createDate`)
SELECT REPLACE(UUID(),'-',''), 'container.gpu.video.ram.size.nvidia',
v_identity_uuid, v_identity_type, v_value, NOW(), NOW() FROM DUAL WHERE NOT
EXISTS (SELECT 1 FROM `QuotaVO` q WHERE
q.`name`='container.gpu.video.ram.size.nvidia' AND
q.`identityUuid`=v_identity_uuid AND q.`identityType`=v_identity_type); keep the
rest of the cursor logic and the final DROP PROCEDURE IF EXISTS call.
Comment on lines
+205
to
+228
| DECLARE cur CURSOR FOR | ||
| SELECT identityUuid, identityType, value | ||
| FROM QuotaVO | ||
| WHERE name = 'container.gpu.video.ram.size'; | ||
| DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE; | ||
|
|
||
| OPEN cur; | ||
| read_loop: LOOP | ||
| FETCH cur INTO v_identity_uuid, v_identity_type, v_value; | ||
| IF done THEN | ||
| LEAVE read_loop; | ||
| END IF; | ||
|
|
||
| INSERT IGNORE INTO QuotaVO (uuid, name, identityUuid, identityType, value, lastOpDate, createDate) | ||
| VALUES ( | ||
| REPLACE(UUID(), '-', ''), | ||
| 'container.gpu.video.ram.size.nvidia', | ||
| v_identity_uuid, | ||
| v_identity_type, | ||
| v_value, | ||
| NOW(), | ||
| NOW() | ||
| ); | ||
| END LOOP; |
There was a problem hiding this comment.
迁移过程内的表/列名同样需要反引号
过程里的 QuotaVO 与字段名同样未加反引号，建议按规范补齐，避免保留字冲突与跨版本兼容问题。
🛠️ 建议修改
- DECLARE cur CURSOR FOR
- SELECT identityUuid, identityType, value
- FROM QuotaVO
- WHERE name = 'container.gpu.video.ram.size';
+ DECLARE cur CURSOR FOR
+ SELECT `identityUuid`, `identityType`, `value`
+ FROM `QuotaVO`
+ WHERE `name` = 'container.gpu.video.ram.size';
...
- INSERT IGNORE INTO QuotaVO (uuid, name, identityUuid, identityType, value, lastOpDate, createDate)
+ INSERT IGNORE INTO `QuotaVO` (`uuid`, `name`, `identityUuid`, `identityType`, `value`, `lastOpDate`, `createDate`)As per coding guidelines, 所有表名和列名必须使用反引号包裹（例如：WHERE system = 1），以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| DECLARE cur CURSOR FOR | |
| SELECT identityUuid, identityType, value | |
| FROM QuotaVO | |
| WHERE name = 'container.gpu.video.ram.size'; | |
| DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE; | |
| OPEN cur; | |
| read_loop: LOOP | |
| FETCH cur INTO v_identity_uuid, v_identity_type, v_value; | |
| IF done THEN | |
| LEAVE read_loop; | |
| END IF; | |
| INSERT IGNORE INTO QuotaVO (uuid, name, identityUuid, identityType, value, lastOpDate, createDate) | |
| VALUES ( | |
| REPLACE(UUID(), '-', ''), | |
| 'container.gpu.video.ram.size.nvidia', | |
| v_identity_uuid, | |
| v_identity_type, | |
| v_value, | |
| NOW(), | |
| NOW() | |
| ); | |
| END LOOP; | |
| DECLARE cur CURSOR FOR | |
| SELECT `identityUuid`, `identityType`, `value` | |
| FROM `QuotaVO` | |
| WHERE `name` = 'container.gpu.video.ram.size'; | |
| DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE; | |
| OPEN cur; | |
| read_loop: LOOP | |
| FETCH cur INTO v_identity_uuid, v_identity_type, v_value; | |
| IF done THEN | |
| LEAVE read_loop; | |
| END IF; | |
| INSERT IGNORE INTO `QuotaVO` (`uuid`, `name`, `identityUuid`, `identityType`, `value`, `lastOpDate`, `createDate`) | |
| VALUES ( | |
| REPLACE(UUID(), '-', ''), | |
| 'container.gpu.video.ram.size.nvidia', | |
| v_identity_uuid, | |
| v_identity_type, | |
| v_value, | |
| NOW(), | |
| NOW() | |
| ); | |
| END LOOP; |
🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.6__schema.sql` around lines 205 - 228, Wrap all table
and column identifiers in the migration procedure with backticks to avoid
reserved-word conflicts: update the DECLARE cur CURSOR FOR SELECT to use
`QuotaVO` and backticked columns (`identityUuid`, `identityType`, `value`,
`name`), update the WHERE clause to `WHERE \`name\` =
'container.gpu.video.ram.size'`, and change the INSERT IGNORE INTO to `INSERT
IGNORE INTO \`QuotaVO\` (\`uuid\`, \`name\`, \`identityUuid\`, \`identityType\`,
\`value\`, \`lastOpDate\`, \`createDate\`) VALUES (...)` (also backtick the
literal column names such as `container.gpu.video.ram.size.nvidia` where used);
ensure every occurrence of QuotaVO and its columns in this procedure (cursor,
FETCH, INSERT) is backticked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: ZSTAC-73546
Target: 5.5.6
sync from gitlab !9182