<fix>[storage]: fix unstable DeleteSnapShotGroup2Case#4037
Open
MatheMatrix wants to merge 1 commit into
Open
Conversation
Walkthrough在 VolumeSnapshotBase 中：1) changeStatus 先 reload 最新实体，若存在则通过按 uuid 的条件 SQL 仅更新 status 并同步内存；2) 删除主存快照完成时同样 reload 后通过条件 SQL 仅清空主存相关字段并回复消息。 变更清单快照状态与主存字段更新
序列图（已在隐藏审查栈中为状态更新层提供序列图；总体变更为局部条件更新，故无需额外图示。） 代码审查工作量评估🎯 4 (Complex) | ⏱️ ~45 分钟 兔子的诗
🚥 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 |
Collaborator
|
Comment from yaohua.wu: Review: MR !9933 — ZSV-12287变更概述：将 结论：方案正确，精准命中 Jira 中分析的根因—— 🟡 Warning
🟢 Suggestion
Coverage / 说明
Verdict: APPROVED方案正确，根治性优于重试。 🤖 Robot Reviewer |
Resolves: ZSV-12287 Related: ZSV-5936 Change-Id: I627364726d796a7a67666162637065687263676c
MatheMatrix
force-pushed
the
sync/wenhao.zhang/c-0
branch
from
fd55fb2 to
95aa779
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
🧹 Nitpick comments (1)
storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotBase.java (1)
262-267: ⚡ Quick win清库后顺手同步
self，避免内存态滞后。这里已经把数据库里的
primaryStorageInstallPath和primaryStorageUuid清空了，但self还保留旧值。建议和changeStatus()一样把内存对象同步一下，避免后续复用这个对象时继续带着过期的主存信息。♻️ 建议修改
SQL.New(VolumeSnapshotVO.class) .eq(VolumeSnapshotVO_.uuid, latest.getUuid()) .set(VolumeSnapshotVO_.primaryStorageInstallPath, null) .set(VolumeSnapshotVO_.primaryStorageUuid, null) .update(); + latest.setPrimaryStorageInstallPath(null); + latest.setPrimaryStorageUuid(null); + self = latest; bus.reply(msg, dreply);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotBase.java` around lines 262 - 267, The database fields primaryStorageInstallPath and primaryStorageUuid are being cleared via SQL.New(...) but the in-memory VolumeSnapshotBase object (self) still holds stale values; after the SQL update in VolumeSnapshotBase (the block using SQL.New(VolumeSnapshotVO.class).eq(...).set(...).update()), set self.primaryStorageInstallPath = null and self.primaryStorageUuid = null (mirroring how changeStatus() syncs the in-memory VO) so the memory state matches the DB and prevents reuse of outdated primary storage info.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotBase.java`:
- Line 217: The debug log message incorrectly refers to a "volume snapshot
group" while the code is handling a VolumeSnapshotVO; update the logger.debug
calls in VolumeSnapshotBase (the statements that currently log "volume snapshot
group[uuid=%s] is not found") to use the correct object name such as "volume
snapshot" (or include VolumeSnapshotVO) so that missing snapshot and missing
snapshot-group messages are not confused; locate the logger.debug occurrences
around the null-checks for VolumeSnapshotVO and replace the string accordingly.
---
Nitpick comments:
In `@storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotBase.java`:
- Around line 262-267: The database fields primaryStorageInstallPath and
primaryStorageUuid are being cleared via SQL.New(...) but the in-memory
VolumeSnapshotBase object (self) still holds stale values; after the SQL update
in VolumeSnapshotBase (the block using
SQL.New(VolumeSnapshotVO.class).eq(...).set(...).update()), set
self.primaryStorageInstallPath = null and self.primaryStorageUuid = null
(mirroring how changeStatus() syncs the in-memory VO) so the memory state
matches the DB and prevents reuse of outdated primary storage info.
🪄 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: 6a5e7664-d748-4e5f-889e-eaddca2f84f5
📒 Files selected for processing (1)
storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotBase.java
| String uuid = self.getUuid(); | ||
| VolumeSnapshotVO latest = dbf.reload(self); | ||
| if (latest == null) { | ||
| logger.debug(String.format("volume snapshot group[uuid=%s] is not found", uuid)); |
There was a problem hiding this comment.
修正调试日志里的对象名称。
这里处理的是 VolumeSnapshotVO，当前日志写成了 “volume snapshot group”，并发删除排障时会把“快照不存在”和“快照组不存在”混淆。
📝 建议修改
- logger.debug(String.format("volume snapshot group[uuid=%s] is not found", uuid));
+ logger.debug(String.format("volume snapshot[uuid=%s] is not found", uuid));Also applies to: 257-257
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@storage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotBase.java` at
line 217, The debug log message incorrectly refers to a "volume snapshot group"
while the code is handling a VolumeSnapshotVO; update the logger.debug calls in
VolumeSnapshotBase (the statements that currently log "volume snapshot
group[uuid=%s] is not found") to use the correct object name such as "volume
snapshot" (or include VolumeSnapshotVO) so that missing snapshot and missing
snapshot-group messages are not confused; locate the logger.debug occurrences
around the null-checks for VolumeSnapshotVO and replace the string accordingly.
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-12287
Related: ZSV-5936
Change-Id: I627364726d796a7a67666162637065687263676c
sync from gitlab !9933