fix(ai): add DB migration for per-vendor GPU quota#3353
fix(ai): add DB migration for per-vendor GPU quota#3353zstack-robot-1 wants to merge 25 commits into5.5.6from
Conversation
…isk offering Resolves: ZSTAC-74683 Change-Id: Id0339ed0221e92e506f60745cde972cc3ee6d9ae
When anti-split-brain check selects a disconnected MDS node, the HTTP call now times out after 30s instead of 5+ minutes, and automatically retries the next available MDS via tryNext mechanism. Resolves: ZSTAC-80595 Change-Id: I1be80f1b70cad1606eb38d1f0078c8f2781e6941
When MN restarts during a destroy operation, the hypervisor may report the VM as Stopped. Without this transition, the state machine throws an exception and the VM stays stuck in Destroying state forever. Resolves: ZSTAC-80620 Change-Id: I037edba70d145a44a88ce0d3573089182fedb162
…pacity Resolves: ZSTAC-79709 Change-Id: I45a2133bbb8c51c25ae3549d59e588976192a08d
Resolves: ZSTAC-78989 Change-Id: I0fe3a56ab724978944c69afadaab7ff7353e4c0f
Resolves: ZSTAC-82153 Change-Id: Ib51c2e21553277416d1a9444be55aca2aa4b2fc4
…fterAddIpAddress to prevent NPE during rollback Resolves: ZSTAC-81741 Change-Id: I53bcf20a10306afc7b6172da294d347b74e6c41f
Resolves: ZSTAC-81182 Change-Id: Id1bb642154dc66ae9995dcc4d9fc00cdce9bcaf8
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough新增 MySQL/MariaDB 数据库迁移脚本：定义并执行存储过程，将全局 GPU 配额 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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)
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.
| 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).
There was a problem hiding this comment.
Comment from ye.zou:
ZStack SQL migrations conventionally do not use backticks around table/column names. All names here are safe (no reserved words). The NULL handling point is noted but type column is NOT NULL in schema. Resolving per existing convention.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check the usage patterns of backticks in the current V5.5.6__schema.sql file
echo "=== Backtick usage patterns in V5.5.6__schema.sql ==="
echo ""
echo "Lines with backticks around table/column names:"
grep -n '`' conf/db/upgrade/V5.5.6__schema.sql | head -20
echo ""
echo "---"
echo ""
echo "Lines with DELETE/UPDATE/INSERT without backticks:"
grep -nE '^(DELETE|UPDATE|INSERT)' conf/db/upgrade/V5.5.6__schema.sql | grep -v '`'
# 2. Search for PciDeviceSpecVO schema definition to check type column constraint
echo ""
echo "=== Searching for PciDeviceSpecVO schema definition ==="
rg -n "CREATE TABLE.*PciDeviceSpecVO" -A 20
echo ""
rg -n "type.*NOT NULL" --type=sql | grep -i pcidevicespec
# 3. Look for any ALTER TABLE statements that define the type column
echo ""
echo "=== Searching for type column definition in PciDeviceSpecVO ==="
rg -n "PciDeviceSpecVO.*type" --type=sql -C 2
Length of output: 4218
@zstack-robot-2 Thank you for the clarification. You're absolutely right about the type column being NOT NULL (confirmed in V3.5.0.1__schema.sql), so the NULL handling concern doesn't apply here.
Regarding backticks, I checked the current V5.5.6__schema.sql file and found that the majority of statements actually do use backticks (e.g., lines 1, 4, 11, 48-87, 98-120), with only a few exceptions. The coding guideline specifically states "所有表名和列名必须使用反引号包裹...以避免 MySQL 8.0 / GreatSQL 保留关键字冲突", which suggests this is a standard requirement rather than an optional convention.
That said, I understand you're resolving based on your assessment that these specific identifiers are safe. I'll note your decision.
🔥 Failed to add learnings
Learnings cannot be added due to server error. Please try again.
conf/db/upgrade/V5.5.6__schema.sql
Outdated
| 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.
There was a problem hiding this comment.
Comment from ye.zou:
ZStack SQL migrations conventionally do not use backticks around table/column names. All names here are safe (no reserved words). The NULL handling point is noted but type column is NOT NULL in schema. Resolving per existing convention.
There was a problem hiding this comment.
@MatheMatrix Thank you for the clarification. However, I need to respectfully point out a discrepancy:
The user-provided coding guidelines for this repository explicitly state:
**/*.sql: 所有表名和列名必须使用反引号包裹（例如：WHEREsystem= 1），以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误
Translation: "All table and column names must be wrapped with backticks (e.g., WHERE system = 1) to avoid syntax errors from MySQL 8.0 / GreatSQL reserved keyword conflicts."
Additionally, the same file (V5.5.6__schema.sql) extensively uses backticks throughout:
- Line 41:
UPDATE `zstack`.`BareMetal2InstanceProvisionNicVO` - Line 44:
ALTER TABLE `zstack`.`BareMetal2InstanceProvisionNicVO` - Line 78:
DELETE FROM `zstack`.`SystemTagVO` - Line 105:
UPDATE `zstack`.`GpuDeviceSpecVO` - Lines 106, 186, 189, and many others
While the current names may be safe, the guideline emphasizes forward compatibility with MySQL 8.0 and GreatSQL, where reserved keyword lists may expand.
Regarding the other concerns I raised:
-
Missing
DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor;beforeCREATE PROCEDURE— this makes the migration non-idempotent if interrupted and re-run. -
INSERT IGNOREdoesn't prevent duplicate business logic — with auto-generated UUIDs, the same(name, identityUuid, identityType)tuple can be inserted multiple times on re-execution.
Could you clarify whether you'd like to:
- Follow the explicit coding guideline and add backticks + address the idempotency issues, or
- Document an exception to the guideline for this specific migration?
🐰
conf/db/upgrade/V5.5.6__schema.sql
Outdated
| 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.
| 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.
There was a problem hiding this comment.
Comment from ye.zou:
ZStack SQL migrations conventionally do not use backticks around table/column names. All names here are safe (no reserved words). The NULL handling point is noted but type column is NOT NULL in schema. Resolving per existing convention.
There was a problem hiding this comment.
@zstack-robot-1 Thank you for the clarification about ZStack's convention. I understand you're following the established practice in your migrations.
However, I notice there's a direct conflict with the documented coding guideline for this repository, which explicitly states:
所有表名和列名必须使用反引号包裹（例如：WHERE
system= 1），以避免 MySQL 8.0 / GreatSQL 保留关键字冲突导致的语法错误
(All table names and column names must be wrapped in backticks to avoid syntax errors caused by MySQL 8.0 / GreatSQL reserved keyword conflicts)
Looking at the existing file, there's also mixed usage—some statements use backticks (ALTER TABLE \zstack`.`GpuDeviceVO`), while others don't (DELETE g FROM GpuDeviceSpecVO g`).
If the team's actual convention differs from the documented guideline, you may want to update the guideline to reflect current practice, or vice versa. This would help maintain consistency for future contributions.
I'll defer to your judgment on this matter.
MatheMatrix
force-pushed
the
sync/ye.zou/fix/ZSTAC-73546
branch
from
ba1f008 to
883a282
Compare
<fix>[vm]: add Destroying->Stopped state transition See merge request zstackio/zstack!9156
<fix>[i18n]: improve snapshot error message for unattached volume See merge request zstackio/zstack!9192
<fix>[zbs]: reduce mds connect timeout and enable tryNext for volume clients See merge request zstackio/zstack!9153
<fix>[vm]: use max of virtual and actual size for root disk allocation See merge request zstackio/zstack!9155
…talling In dual management node scenarios, concurrent modifications to the consistent hash ring from heartbeat reconciliation and canonical event callbacks can cause NodeHash/Nodes inconsistency, leading to message routing failures and task timeouts. Fix: (1) synchronized all ResourceDestinationMakerImpl methods to ensure atomic nodeHash+nodes updates, (2) added lifecycleLock in ManagementNodeManagerImpl to serialize heartbeat reconciliation with event callbacks, (3) added two-round delayed confirmation before removing nodes from hash ring to avoid race with NodeJoin events. Resolves: ZSTAC-77711 Change-Id: I3d33d53595dd302784dff17417a5b25f2d0f3426
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 6-7: Add idempotency by dropping the stored procedure before
creating it: ensure a DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor() is
executed immediately before the CREATE PROCEDURE MigrateGpuQuotaPerVendor()
statement so repeated/partial upgrade runs won't fail due to the procedure
already existing.
🧹 Nitpick comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)
8-36: 可用集合式 INSERT 简化游标循环，提升性能与可读性。当前逐行游标处理可以改为一次性
INSERT … SELECT，保留在存储过程中即可满足历史数据处理要求。♻️ 建议重构（示例）
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; + INSERT IGNORE INTO `QuotaVO` (`uuid`, `name`, `identityUuid`, `identityType`, `value`, `lastOpDate`, `createDate`) + SELECT + REPLACE(UUID(), '-', ''), + 'container.gpu.video.ram.size.nvidia', + `identityUuid`, + `identityType`, + `value`, + NOW(), + NOW() + FROM `QuotaVO` + WHERE `name` = 'container.gpu.video.ram.size'; END$$
| CREATE PROCEDURE MigrateGpuQuotaPerVendor() | ||
| BEGIN |
There was a problem hiding this comment.
建议在创建存储过程前先 DROP，避免升级重跑失败。
若迁移在中途失败并重试，CREATE PROCEDURE 可能因已存在而报错中断升级。建议增加 DROP PROCEDURE IF EXISTS 以保证幂等性。
🔧 建议修改
DELIMITER $$
+DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor$$
+
CREATE PROCEDURE MigrateGpuQuotaPerVendor()
BEGIN📝 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.
| CREATE PROCEDURE MigrateGpuQuotaPerVendor() | |
| BEGIN | |
| DELIMITER $$ | |
| DROP PROCEDURE IF EXISTS MigrateGpuQuotaPerVendor$$ | |
| CREATE PROCEDURE MigrateGpuQuotaPerVendor() | |
| BEGIN |
🤖 Prompt for AI Agents
In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 6 - 7, Add idempotency by
dropping the stored procedure before creating it: ensure a DROP PROCEDURE IF
EXISTS MigrateGpuQuotaPerVendor() is executed immediately before the CREATE
PROCEDURE MigrateGpuQuotaPerVendor() statement so repeated/partial upgrade runs
won't fail due to the procedure already existing.
<fix>[network]: filter reserved IPs in getFreeIp See merge request zstackio/zstack!9170
<fix>[ceph]: apply over-provisioning ratio when releasing snapshot capacity See merge request zstackio/zstack!9162
<fix>[network]: add null check for L3 network system tags in API interceptor See merge request zstackio/zstack!9169
MatheMatrix
force-pushed
the
sync/ye.zou/fix/ZSTAC-73546
branch
from
91c21d6 to
cf4df42
Compare
The mdsUrls field in ExternalPrimaryStorage config contains user:password@host format credentials. Add desensitization to mask credentials as ***@host in API/CLI output. Resolves: ZSTAC-80664 Change-Id: I94bdede5a1b52eb039de70efb5458693484405f7
Add ORG_ZSTACK_STORAGE_BACKUP_CANCEL_TIMEOUT constant to CloudOperationsErrorCode for use in premium volumebackup module. Resolves: ZSTAC-82195 Change-Id: Ibc405876e1171b637cf76b91a6822574fb6e7811
<fix>[core]: synchronize consistent hash ring to prevent dual-MN race condition See merge request zstackio/zstack!9154
SyncTaskFuture constructor calls Context.current() unconditionally, triggering ServiceLoader for ContextStorageProvider even when telemetry is disabled. If sentry-opentelemetry-bootstrap jar is on classpath, ServiceLoader fails with "not a subtype" due to ClassLoader isolation in Tomcat, throwing ServiceConfigurationError (extends Error) that escapes all catch(Exception) blocks. 1. Why is this change necessary? MN startup crashes with ORG_ZSTACK_CORE_WORKFLOW_10001 because Context.current() triggers ServiceLoader unconditionally in SyncTaskFuture constructor, even when telemetry is disabled. 2. How does it address the problem? Only call Context.current() when isTelemetryEnabled() returns true, matching the existing guard pattern used in other DispatchQueueImpl code paths (lines 351, 1069). 3. Are there any side effects? None. When telemetry is disabled, parentContext was never used. # Summary of changes (by module): - core/thread: conditionalize Context.current() in SyncTaskFuture Related: ZSTAC-82275 Change-Id: I5c0e1f15769c746c630028a29df8cf1815620608
<fix>[thread]: guard Context.current() with telemetry check See merge request zstackio/zstack!9202
<fix>[loadBalancer]: block SLB deletion during grayscale upgrade See merge request zstackio/zstack!9187
<fix>[volumebackup]: add backup cancel timeout error code See merge request zstackio/zstack!9200
<fix>[core]: add @nologging to sensitive config fields See merge request zstackio/zstack!9171
MatheMatrix
force-pushed
the
sync/ye.zou/fix/ZSTAC-73546
branch
from
cf4df42 to
4a03933
Compare
MatheMatrix
force-pushed
the
sync/ye.zou/fix/ZSTAC-73546
branch
3 times, most recently
from
9c5f13c to
77029dd
Compare
Migrate existing container.gpu.video.ram.size QuotaVO rows to per-vendor format (e.g. container.gpu.video.ram.size.nvidia). Use INSERT IGNORE and idempotent DROP/CREATE PROCEDURE pattern. Resolves: ZSTAC-73546 Change-Id: If057b328bfd3db5265c15ba0185025bcaa41de07
MatheMatrix
force-pushed
the
sync/ye.zou/fix/ZSTAC-73546
branch
from
77029dd to
1988f21
Compare
5 participants
Resolves: ZSTAC-73546
Target: 5.5.6
sync from gitlab !9182