<feature>[snapshot]: idempotent commit-delete via status=Deleting latch#4060
Open
zstack-robot-2 wants to merge 1 commit into
Open
<feature>[snapshot]: idempotent commit-delete via status=Deleting latch#4060zstack-robot-2 wants to merge 1 commit into
zstack-robot-2 wants to merge 1 commit into
Conversation
Resolves: ZSV-10538 Change-Id: Ib3589ce5551984fa25b1562c745474437f7d1669
总览此 PR 实现 ZSV-10538，将快照删除改造为幂等可中断恢复模式。通过添加 变更快照删除幂等性与中断恢复
🎯 3 (中等复杂度) | ⏱️ ~25 分钟
🚥 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 |
![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/SnapshotReconcileOnStart.java (1)
33-49: 建议补充可监控指标而不仅是日志告警当前实现可用，但建议同步上报一个启动对账指标（如 deleting 快照数量、最早 deletingSince），便于监控系统做阈值告警，避免仅靠日志被动发现。
🤖 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/SnapshotReconcileOnStart.java` around lines 33 - 49, The code in SnapshotReconcileOnStart currently logs snapshots stuck in Deleting (variable stuck) but doesn't emit metrics; modify SnapshotReconcileOnStart to report at least two monitoring metrics when stuck is non-empty: a gauge for the count of deleting snapshots (stuck.size()) and a timestamp or age metric representing the earliest deletingSince among the VolumeSnapshotVOs, in addition to the existing logger.warn calls; use the project metric/reporting API (e.g., your Metrics or MetricReporter helper) to send a numeric gauge named like snapshot.deleting.count and snapshot.deleting.oldest_timestamp (or snapshot.deleting.max_age) so alerting systems can pick it up, ensuring the metric emission occurs in the same conditional block that handles stuck and references stuck, VolumeSnapshotVO.getDeletingSince(), and SnapshotReconcileOnStart.
🤖 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 `@conf/i18n/messages_zh_CN.properties`:
- Line 4834: Update the translation value for the key starting with
"snapshot[uuid\:%s,\ name\:%s]\ is\ in\ Deleting\ status,\ operation[%s]\ is\
rejected;\ allowed\ messages%s" so the Chinese text inserts a separator before
the placeholder {3}; change "当前状态允许的消息列表{3}" to "当前状态允许的消息列表：{3}" to avoid the
placeholder being concatenated with the preceding text.
---
Nitpick comments:
In
`@storage/src/main/java/org/zstack/storage/snapshot/SnapshotReconcileOnStart.java`:
- Around line 33-49: The code in SnapshotReconcileOnStart currently logs
snapshots stuck in Deleting (variable stuck) but doesn't emit metrics; modify
SnapshotReconcileOnStart to report at least two monitoring metrics when stuck is
non-empty: a gauge for the count of deleting snapshots (stuck.size()) and a
timestamp or age metric representing the earliest deletingSince among the
VolumeSnapshotVOs, in addition to the existing logger.warn calls; use the
project metric/reporting API (e.g., your Metrics or MetricReporter helper) to
send a numeric gauge named like snapshot.deleting.count and
snapshot.deleting.oldest_timestamp (or snapshot.deleting.max_age) so alerting
systems can pick it up, ensuring the metric emission occurs in the same
conditional block that handles stuck and references stuck,
VolumeSnapshotVO.getDeletingSince(), and SnapshotReconcileOnStart.
🪄 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: f7e70bd7-0052-446f-9751-45f357391b21
⛔ Files ignored due to path filters (1)
conf/springConfigXml/volumeSnapshot.xmlis excluded by!**/*.xml
📒 Files selected for processing (9)
conf/db/upgrade/V4.8.0.8__schema.sqlconf/i18n/messages_en_US.propertiesconf/i18n/messages_zh_CN.propertiesheader/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotAO.javaheader/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotAO_.javastorage/src/main/java/org/zstack/storage/snapshot/SnapshotReconcileOnStart.javastorage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotErrors.javastorage/src/main/java/org/zstack/storage/snapshot/VolumeSnapshotTreeBase.javastorage/src/main/java/org/zstack/storage/snapshot/group/VolumeSnapshotGroupBase.java
| LessThan = 小于 | ||
| LessThanOrEqualTo = 小于等于 | ||
| resource[%s]\ doesn't\ support\ zwatch\ return\ with\ clause = 资源[{0}]不支持ZWatch Return WITH子句 | ||
| snapshot[uuid\:%s,\ name\:%s]\ is\ in\ Deleting\ status,\ operation[%s]\ is\ rejected;\ allowed\ messages%s = 快照[uuid:{0}, name:{1}]处于Deleting状态，拒绝操作[{2}]；当前状态允许的消息列表{3} |
There was a problem hiding this comment.
建议在“消息列表”前补充分隔符，避免占位符直连文本。
当前中文文案为“当前状态允许的消息列表{3}”，建议改为“当前状态允许的消息列表：{3}”，提升用户阅读体验。
🤖 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 `@conf/i18n/messages_zh_CN.properties` at line 4834, Update the translation
value for the key starting with "snapshot[uuid\:%s,\ name\:%s]\ is\ in\
Deleting\ status,\ operation[%s]\ is\ rejected;\ allowed\ messages%s" so the
Chinese text inserts a separator before the placeholder {3}; change
"当前状态允许的消息列表{3}" to "当前状态允许的消息列表：{3}" to avoid the placeholder being
concatenated with the preceding text.
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-10538
Change-Id: Ib3589ce5551984fa25b1562c745474437f7d1669
sync from gitlab !9958