<fix>[cascade]: cleanup orphan templated vm and snapshot tree#3851
Open
MatheMatrix wants to merge 1 commit intozsv_5.0.0from
Open
<fix>[cascade]: cleanup orphan templated vm and snapshot tree#3851MatheMatrix wants to merge 1 commit intozsv_5.0.0from
MatheMatrix wants to merge 1 commit intozsv_5.0.0from
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough在级联删除清理流程中补充孤立数据回溯删除：虚拟机删除新增对模板化虚拟机相关表的孤立一致性批量清理；卷快照删除路径补充了对 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
MatheMatrix
force-pushed
the
sync/tao.gan/ZSV-11769@@2
branch
from
5115097 to
6d26ff1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmCascadeExtension.java`:
- Around line 307-321: The current code loads all orphan UUIDs into memory via
sql(...).list() and then issues three hardDelete() calls with large IN
predicates (TemplatedVmInstanceRefVO, TemplatedVmInstanceCacheVO,
TemplatedVmInstanceVO), which can cause memory/SQL parameter issues; change each
delete to a single pure-SQL subquery delete that removes rows where
templatedVmInstanceUuid/uuid is in (select uuid from TemplatedVmInstanceVO t
where t.uuid not in (select vm.uuid from VmInstanceEO vm)) so you avoid building
the orphanUuids list and large IN parameter lists—update the delete logic in
VmCascadeExtension to run those three deletes as SQL subquery deletes
referencing TemplatedVmInstanceVO, TemplatedVmInstanceRefVO and
TemplatedVmInstanceCacheVO instead of using orphanUuids.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 06bcec0e-794f-4304-b309-5c93e76c7c27
📒 Files selected for processing (2)
compute/src/main/java/org/zstack/compute/vm/VmCascadeExtension.javastorage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotCascadeExtension.java
🚧 Files skipped from review as they are similar to previous changes (1)
- storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotCascadeExtension.java
Comment on lines
+307
to
+321
| List<String> orphanUuids = sql("select t.uuid from TemplatedVmInstanceVO t" + | ||
| " where t.uuid not in (select vm.uuid from VmInstanceEO vm)", String.class).list(); | ||
| if (orphanUuids.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| sql(TemplatedVmInstanceRefVO.class) | ||
| .in(TemplatedVmInstanceRefVO_.templatedVmInstanceUuid, orphanUuids) | ||
| .hardDelete(); | ||
| sql(TemplatedVmInstanceCacheVO.class) | ||
| .in(TemplatedVmInstanceCacheVO_.templatedVmInstanceUuid, orphanUuids) | ||
| .hardDelete(); | ||
| sql(TemplatedVmInstanceVO.class) | ||
| .in(TemplatedVmInstanceVO_.uuid, orphanUuids) | ||
| .hardDelete(); |
There was a problem hiding this comment.
避免先全量拉取 orphan UUID 再做 IN 批删。
Line 307-321 当前实现会把孤儿 UUID 全量加载到内存，并在 3 次删除中展开大 IN 条件。孤儿数据规模大时，容易触发内存压力或 SQL/参数长度上限，导致 cleanup 阶段超时或失败。
建议改为纯 SQL 子查询删除（避免全量 list + IN）
- List<String> orphanUuids = sql("select t.uuid from TemplatedVmInstanceVO t" +
- " where t.uuid not in (select vm.uuid from VmInstanceEO vm)", String.class).list();
- if (orphanUuids.isEmpty()) {
- return;
- }
-
- sql(TemplatedVmInstanceRefVO.class)
- .in(TemplatedVmInstanceRefVO_.templatedVmInstanceUuid, orphanUuids)
- .hardDelete();
- sql(TemplatedVmInstanceCacheVO.class)
- .in(TemplatedVmInstanceCacheVO_.templatedVmInstanceUuid, orphanUuids)
- .hardDelete();
- sql(TemplatedVmInstanceVO.class)
- .in(TemplatedVmInstanceVO_.uuid, orphanUuids)
- .hardDelete();
+ sql("delete from TemplatedVmInstanceRefVO ref" +
+ " where ref.templatedVmInstanceUuid in (" +
+ " select t.uuid from TemplatedVmInstanceVO t" +
+ " where not exists (select 1 from VmInstanceEO vm where vm.uuid = t.uuid)" +
+ " )").execute();
+
+ sql("delete from TemplatedVmInstanceCacheVO cache" +
+ " where cache.templatedVmInstanceUuid in (" +
+ " select t.uuid from TemplatedVmInstanceVO t" +
+ " where not exists (select 1 from VmInstanceEO vm where vm.uuid = t.uuid)" +
+ " )").execute();
+
+ sql("delete from TemplatedVmInstanceVO t" +
+ " where not exists (select 1 from VmInstanceEO vm where vm.uuid = t.uuid)").execute();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compute/src/main/java/org/zstack/compute/vm/VmCascadeExtension.java` around
lines 307 - 321, The current code loads all orphan UUIDs into memory via
sql(...).list() and then issues three hardDelete() calls with large IN
predicates (TemplatedVmInstanceRefVO, TemplatedVmInstanceCacheVO,
TemplatedVmInstanceVO), which can cause memory/SQL parameter issues; change each
delete to a single pure-SQL subquery delete that removes rows where
templatedVmInstanceUuid/uuid is in (select uuid from TemplatedVmInstanceVO t
where t.uuid not in (select vm.uuid from VmInstanceEO vm)) so you avoid building
the orphanUuids list and large IN parameter lists—update the delete logic in
VmCascadeExtension to run those three deletes as SQL subquery deletes
referencing TemplatedVmInstanceVO, TemplatedVmInstanceRefVO and
TemplatedVmInstanceCacheVO instead of using orphanUuids.
Resolves: ZSV-11769 Change-Id: I616d676e647a656162756e666b7862747668736a
MatheMatrix
force-pushed
the
sync/tao.gan/ZSV-11769@@2
branch
from
6d26ff1 to
ad19409
Compare
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: ZSV-11769
Change-Id: I616d676e647a656162756e666b7862747668736a
sync from gitlab !9723