<fix>[zbs]: sync cow snapshot size in scheduled volume resource sync#4446
Open
MatheMatrix wants to merge 1 commit into
Open
<fix>[zbs]: sync cow snapshot size in scheduled volume resource sync#4446MatheMatrix wants to merge 1 commit into
MatheMatrix wants to merge 1 commit into
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
Warning
|
| Layer / File(s) | Summary |
|---|---|
资源统计与消息契约 header/src/main/java/org/zstack/header/storage/..., header/src/main/java/org/zstack/header/volume/... |
新增通用资源统计模型、快照模式及卷与快照资源大小同步消息/回复，并调整统计类型继承关系。 |
快照能力配置 plugin/*/src/main/java/..., simulator/... |
各主存储根据卷类型配置快照能力与模式，ZBS 使用 COPY_ON_WRITE，其他相关实现使用 REDIRECT_ON_WRITE。 |
统计查询与资源同步适配 storage/src/main/java/org/zstack/storage/..., plugin/*/src/main/java/... |
批量统计改用 StorageResourceStats，同步消息和回复扩展为卷与快照大小，并完成主存储后端适配。 |
活动卷快照大小同步 storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java |
缓存快照能力，按主存储发送资源大小请求，并更新卷及快照实际大小。 |
集成测试验证 test/src/test/groovy/..., scheduled-volume-size-sync-case.md |
更新资源大小同步断言，并验证定时同步中的卷、快照大小及能力缓存。 |
Estimated code review effort: 4 (Complex) | ~60 minutes
Sequence Diagram(s)
sequenceDiagram
participant VolumeManagerImpl
participant PrimaryStorage
participant StorageController
VolumeManagerImpl->>PrimaryStorage: 发送卷与快照资源大小请求
PrimaryStorage->>StorageController: 查询资源统计
StorageController-->>PrimaryStorage: 返回卷和快照实际大小
PrimaryStorage-->>VolumeManagerImpl: 返回资源大小映射
VolumeManagerImpl->>VolumeManagerImpl: 更新卷和快照大小
Poem
我是兔子，蹦过容量田，
新消息牵来快照线。
卷的大小排成队，
快照也有新家园。
模式一设，数据甜！
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.78% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 标题简洁且准确概括了本次变更：在定时卷资源同步中同步 COW 快照大小。 |
| Description check | ✅ Passed | 描述与变更内容高度一致，涵盖了快照大小同步、消息重命名、能力查询和批量统计调整。 |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
sync/xiangheng.zhao/fix-zbs-snapshot-capacity-not-update@@3
Comment @coderabbitai help to get the list of available commands.
MatheMatrix
force-pushed
the
sync/xiangheng.zhao/fix-zbs-snapshot-capacity-not-update@@3
branch
2 times, most recently
from
b38b617 to
e8a38f0
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java (1)
1262-1295: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win建议对
returnValue.getVolumes()做空值防护
toVolumeSnapshotStats已经对snapshots为null做了防护，但returnValue.getVolumes()在 Line 1276 直接调用.entrySet()，若 ZBS agent 返回的响应中volumes字段为null（例如异常或不完整响应），会在异步 HTTP 回调线程中抛出 NPE，可能导致该次批量同步请求悬挂而不会走fail分支正常返回错误。🛡️ 建议的空值防护
- List<StorageResourceStats> stats = returnValue.getVolumes().entrySet().stream().map(v -> { + Map<String, Map<String, Long>> volumes = returnValue.getVolumes() == null ? Collections.emptyMap() : returnValue.getVolumes(); + List<StorageResourceStats> stats = volumes.entrySet().stream().map(v -> {🤖 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 `@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java` around lines 1262 - 1295, 在 batchStats 的异步 success 回调中，对 returnValue.getVolumes() 做空值防护，避免直接调用 entrySet()；将 null 视为空集合并返回空的卷统计，同时保留现有快照统计处理和 comp.success 流程。
🧹 Nitpick comments (3)
header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java (1)
4-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
size字段的 getter/setter 类型不一致。
size字段声明为装箱类型Long，但setSize参数却使用原始类型long，与actualSize的Long/Long读写方式不统一，破坏了对称性，也无法通过 setter 显式置空。♻️ 建议统一为装箱类型
- public void setSize(long size) { + public void setSize(Long size) { this.size = size; }Also applies to: 24-30
🤖 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 `@header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java` around lines 4 - 6, 统一 StorageResourceStats 中 size 的访问器类型：将 setSize 的参数从原始类型 long 改为装箱类型 Long，使其与 size 字段及 actualSize 的 getter/setter 保持一致，并支持通过 setter 显式设置 null。plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java (1)
2674-2697: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff卷类型解析与快照能力赋值逻辑在多个主存储实现中重复。
这段“取
volumeType（含回退）→ 按 Data/Root/Memory 分支设置 arrangement/placement/mode→未知类型抛异常”的逻辑与NfsPrimaryStorage.handle(AskVolumeSnapshotCapabilityMsg)、SMPPrimaryStorageBase.handle(AskVolumeSnapshotCapabilityMsg)几乎完全一致，属于跨文件的重复代码，建议抽取为共享静态工具方法（如放入VolumeSnapshotCapability或独立 helper 类），减少重复维护成本。♻️ 抽取共享逻辑示例
public static VolumeSnapshotCapability buildDefaultChainRedirectCapability(String volumeType) { VolumeSnapshotCapability capability = new VolumeSnapshotCapability(); capability.setSupport(true); if (VolumeType.Data.toString().equals(volumeType) || VolumeType.Root.toString().equals(volumeType)) { capability.setArrangementType(VolumeSnapshotArrangementType.CHAIN); capability.setPlacementType(VolumeSnapshotCapability.VolumeSnapshotPlacementType.EXTERNAL); capability.setMode(VolumeSnapshotCapability.VolumeSnapshotMode.REDIRECT_ON_WRITE); } else if (VolumeType.Memory.toString().equals(volumeType)) { capability.setArrangementType(VolumeSnapshotArrangementType.INDIVIDUAL); } else { throw new CloudRuntimeException(String.format("unknown volume type %s", volumeType)); } return capability; }🤖 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 `@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java` around lines 2674 - 2697, Extract the shared capability-building logic from LocalStorageBase.handle, NfsPrimaryStorage.handle, and SMPPrimaryStorageBase.handle into one reusable static helper, such as on VolumeSnapshotCapability. The helper must preserve volume-type handling, including the null fallback before invocation, support=true, Data/Root chain redirect settings, Memory individual arrangement, and unknown-type exception behavior; update all three handlers to call it and only manage the reply.plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java (1)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win为新的资源大小同步接口补充 Javadoc。
请明确 NFS 后端仅填充卷实际大小，避免调用方误以为该接口同时返回快照统计。
建议修改
+ /** + * 批量同步主存储上的卷资源实际大小。 + * NFS 后端仅填充卷实际大小，不返回快照实际大小。 + */ void handle(PrimaryStorageInventory inv, BatchSyncVolumeResourceSizeOnPrimaryStorageMsg msg, ReturnValueCompletion<BatchSyncVolumeResourceSizeOnPrimaryStorageReply> completion);As per path instructions, “接口方法不应有多余的修饰符（例如 public），且必须配有有效的 Javadoc 注释”。
🤖 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 `@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java` at line 33, 为 NFS 后端接口中的 handle 方法补充有效 Javadoc，说明该批量资源大小同步接口仅填充卷的实际大小，不返回快照统计；保持方法现有签名不变，并避免添加多余的访问修饰符。Source: Path instructions
🤖 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
`@header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java`:
- Line 68: 为 PrimaryStorageControllerSvc.batchStats 补充有效 Javadoc，明确 installPaths
表示待统计的卷或快照资源路径，说明 comp 的返回结果为资源统计列表，元素可能是 VolumeStats 或
VolumeSnapshotStats，并注明完成回调的用途。
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java`:
- Around line 3405-3408: 将私有方法 BatchSyncVolumeResourceSizeOnPrimaryStorage
重命名为符合 lowerCamelCase 的 batchSyncVolumeResourceSizeOnPrimaryStorage，并同步更新
handle(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg) 及所有相关调用、方法引用和声明。
In `@storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java`:
- Around line 1038-1043: Update the filter in the snapshot size persistence
stream around snapshotActualSizes so non-null values including 0 are written to
VolumeSnapshotVO_.size. Remove the positive-value restriction while preserving
the existing update flow for each snapshot UUID, ensuring refreshVolume() uses
the current zero size.
- Line 843: 移除围绕 snapshotSizeSyncRequiredCache 的 retainAll(currentCacheKeys)
裁剪逻辑，避免单个主机请求删除其他并发流程仍需使用的全局缓存项。保持已准备的缓存状态可供后续快照大小同步流程（包括 Line 925
对应逻辑）读取，确保不会因缺失回退为 false。
---
Outside diff comments:
In `@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java`:
- Around line 1262-1295: 在 batchStats 的异步 success 回调中，对 returnValue.getVolumes()
做空值防护，避免直接调用 entrySet()；将 null 视为空集合并返回空的卷统计，同时保留现有快照统计处理和 comp.success 流程。
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java`:
- Around line 4-6: 统一 StorageResourceStats 中 size 的访问器类型：将 setSize 的参数从原始类型 long
改为装箱类型 Long，使其与 size 字段及 actualSize 的 getter/setter 保持一致，并支持通过 setter 显式设置 null。
In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`:
- Around line 2674-2697: Extract the shared capability-building logic from
LocalStorageBase.handle, NfsPrimaryStorage.handle, and
SMPPrimaryStorageBase.handle into one reusable static helper, such as on
VolumeSnapshotCapability. The helper must preserve volume-type handling,
including the null fallback before invocation, support=true, Data/Root chain
redirect settings, Memory individual arrangement, and unknown-type exception
behavior; update all three handlers to call it and only manage the reply.
In
`@plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.java`:
- Line 33: 为 NFS 后端接口中的 handle 方法补充有效
Javadoc，说明该批量资源大小同步接口仅填充卷的实际大小，不返回快照统计；保持方法现有签名不变，并避免添加多余的访问修饰符。
🪄 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: f20ce702-71d9-4b82-ad33-6ed89b141324
📒 Files selected for processing (27)
header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.javaheader/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.javaheader/src/main/java/org/zstack/header/storage/primary/AskVolumeSnapshotCapabilityMsg.javaheader/src/main/java/org/zstack/header/storage/primary/PrimaryStorageInventory.javaheader/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.javaheader/src/main/java/org/zstack/header/storage/primary/VolumeSnapshotCapability.javaheader/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.javaheader/src/main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/volume/BatchSyncVolumeSizeOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/volume/VolumeStats.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.javaplugin/expon/src/main/java/org/zstack/expon/ExponStorageController.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageBase.javaplugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/primary/SimulatorPrimaryStorage.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javastorage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/volume/BatchSyncVolumeSizeOnPrimaryStorageReply.java
Comment on lines
+3405
to
+3408
| protected void handle(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg msg) { | ||
| inQueue().name(String.format("batch-sync-volume-size-on-primarystorage-%s", self.getUuid())) | ||
| .asyncBackup(msg) | ||
| .run(chain -> BatchSyncVolumeSizeOnPrimaryStorage(msg, new NoErrorCompletion(chain) { | ||
| .run(chain -> BatchSyncVolumeResourceSizeOnPrimaryStorage(msg, new NoErrorCompletion(chain) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
私有方法名违反 lowerCamelCase 命名规范
私有方法 BatchSyncVolumeResourceSizeOnPrimaryStorage 首字母大写，不符合方法命名应使用 lowerCamelCase 的规范。正好此次重命名，建议一并修正大小写。
♻️ 建议修正
- .run(chain -> BatchSyncVolumeResourceSizeOnPrimaryStorage(msg, new NoErrorCompletion(chain) {
+ .run(chain -> batchSyncVolumeResourceSizeOnPrimaryStorage(msg, new NoErrorCompletion(chain) {- private void BatchSyncVolumeResourceSizeOnPrimaryStorage(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg msg, NoErrorCompletion completion) {
+ private void batchSyncVolumeResourceSizeOnPrimaryStorage(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg msg, NoErrorCompletion completion) {As per path instructions, "方法名、参数名、成员变量和局部变量：使用 lowerCamelCase 风格。"
Also applies to: 3427-3457
🤖 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
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java`
around lines 3405 - 3408, 将私有方法 BatchSyncVolumeResourceSizeOnPrimaryStorage
重命名为符合 lowerCamelCase 的 batchSyncVolumeResourceSizeOnPrimaryStorage，并同步更新
handle(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg) 及所有相关调用、方法引用和声明。
Source: Path instructions
MatheMatrix
force-pushed
the
sync/xiangheng.zhao/fix-zbs-snapshot-capacity-not-update@@3
branch
3 times, most recently
from
bf4b2f7 to
ef27d04
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java (1)
24-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
setSize参数类型与其他字段不一致。
getSize()/setActualSize(Long)均使用包装类型Long，但setSize(long size)使用原始类型long，风格不统一，建议统一为Long以保持一致性。♻️ 建议统一参数类型
- public void setSize(long size) { + public void setSize(Long size) { this.size = size; }🤖 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 `@header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java` around lines 24 - 30, 将 StorageResourceStats 的 setSize 方法参数从原始类型 long 统一为包装类型 Long，与 getSize 和 setActualSize 保持一致；保留现有赋值行为不变。storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java (1)
1917-1924: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win重复条件建议抽取为局部布尔变量。
msg.isWithSnapshot() && msg.getSnapshotUuidInstallPaths() != null（及其取反形式）在第1917行和第1922行重复出现，建议抽取为一个布尔变量以提升可读性并避免后续维护时两处条件不同步。♻️ 建议抽取布尔变量
+ boolean hasSnapshotPaths = msg.isWithSnapshot() && msg.getSnapshotUuidInstallPaths() != null; Map<String, String> snapshotInstallPathToUuids = !msg.isWithSnapshot() || msg.getSnapshotUuidInstallPaths() == null ? Collections.emptyMap() : - msg.getSnapshotUuidInstallPaths().entrySet().stream().collect(Collectors.toMap( - Map.Entry::getValue, Map.Entry::getKey, (k1, k2) -> k1 - )); + Map<String, String> snapshotInstallPathToUuids = !hasSnapshotPaths ? Collections.emptyMap() : + msg.getSnapshotUuidInstallPaths().entrySet().stream().collect(Collectors.toMap( + Map.Entry::getValue, Map.Entry::getKey, (k1, k2) -> k1 + )); Collection<String> installPaths = new ArrayList<>(msg.getVolumeUuidInstallPaths().values()); - if (msg.isWithSnapshot() && msg.getSnapshotUuidInstallPaths() != null) { + if (hasSnapshotPaths) { installPaths.addAll(msg.getSnapshotUuidInstallPaths().values()); }基于路径指令，“if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述”。
🤖 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/addon/primary/ExternalPrimaryStorage.java` around lines 1917 - 1924, 在相关逻辑中抽取局部布尔变量，统一表示“包含快照且快照安装路径不为空”的条件，并在 snapshotInstallPathToUuids 的三元表达式和后续 installPaths 判断中复用该变量，避免重复条件可能产生不一致。Source: Path instructions
🤖 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.
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java`:
- Around line 24-30: 将 StorageResourceStats 的 setSize 方法参数从原始类型 long 统一为包装类型
Long，与 getSize 和 setActualSize 保持一致；保留现有赋值行为不变。
In
`@storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java`:
- Around line 1917-1924: 在相关逻辑中抽取局部布尔变量，统一表示“包含快照且快照安装路径不为空”的条件，并在
snapshotInstallPathToUuids 的三元表达式和后续 installPaths 判断中复用该变量，避免重复条件可能产生不一致。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a8fb88f3-00b6-4b6c-b621-daeb3915b41c
📒 Files selected for processing (31)
header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.javaheader/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.javaheader/src/main/java/org/zstack/header/storage/primary/AskVolumeSnapshotCapabilityMsg.javaheader/src/main/java/org/zstack/header/storage/primary/PrimaryStorageInventory.javaheader/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.javaheader/src/main/java/org/zstack/header/storage/primary/VolumeSnapshotCapability.javaheader/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.javaheader/src/main/java/org/zstack/header/volume/BatchSyncActiveVolumeSizeOnHostReply.javaheader/src/main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/volume/BatchSyncVolumeSizeOnPrimaryStorageReply.javaheader/src/main/java/org/zstack/header/volume/VolumeStats.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.javaplugin/expon/src/main/java/org/zstack/expon/ExponStorageController.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.javaplugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageBackend.javaplugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.javaplugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageBase.javaplugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.javascheduled-volume-size-sync-case.mdsimulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/primary/SimulatorPrimaryStorage.javastorage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.javastorage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ScheduledVolumeSizeSyncCase.groovytest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovytest/src/test/resources/zstack.properties
💤 Files with no reviewable changes (1)
- header/src/main/java/org/zstack/header/volume/BatchSyncVolumeSizeOnPrimaryStorageReply.java
🚧 Files skipped from review as they are similar to previous changes (24)
- header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageInventory.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageHypervisorBackend.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageKvmBackend.java
- header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.java
- header/src/main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageReply.java
- header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
- storage/src/main/java/org/zstack/storage/primary/PrimaryStorageBase.java
- header/src/main/java/org/zstack/header/storage/primary/AskVolumeSnapshotCapabilityMsg.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorageKVMBackend.java
- header/src/main/java/org/zstack/header/storage/primary/VolumeSnapshotCapability.java
- simulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/primary/SimulatorPrimaryStorage.java
- header/src/main/java/org/zstack/header/volume/VolumeStats.java
- plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
- header/src/main/java/org/zstack/header/volume/BatchSyncActiveVolumeSizeOnHostReply.java
- plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
- header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java
- header/src/main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageMsg.java
- plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/SMPPrimaryStorageBase.java
- plugin/nfsPrimaryStorage/src/main/java/org/zstack/storage/primary/nfs/NfsPrimaryStorage.java
- plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java
- plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java
- plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
- test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy
- storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java
MatheMatrix
force-pushed
the
sync/xiangheng.zhao/fix-zbs-snapshot-capacity-not-update@@3
branch
from
ef27d04 to
b0700e5
Compare
ZBS COPY_ON_WRITE snapshot actual size is maintained by the storage backend and may change after snapshot creation. The console reads snapshot size from VolumeSnapshotVO, but the scheduled active-volume size sync refreshed only volume actual size before, so snapshot capacity could stay stale in the database. During scheduled active-volume size sync, only primary storages whose snapshot mode is COPY_ON_WRITE attach snapshot install paths to the batch volume resource size sync request. The reply carries snapshot actual sizes so VolumeSnapshotVO.size can be refreshed together with volume actual size, without adding a manual snapshot size query flow for the console. Rename BatchSyncVolumeSizeOnPrimaryStorageMsg to BatchSyncVolumeResourceSizeOnPrimaryStorageMsg because the scheduled batch sync now covers volume resources and, for COW snapshot backends, snapshot resources. Keep SyncVolumeSize and single-volume stats as volume-only flows. Query snapshot capability by primary storage identity and volume type. PrimaryStorageInventory#getIdentity provides the cache identity: normal primary storages use type, and external primary storages override it with their identity, so VolumeManagerImpl does not depend on external storage VOs or external/non-external type checks. Use StorageResourceStats as the batchStats return parent for VolumeStats and VolumeSnapshotStats. Batch stats accepts one installPaths collection; ZBS detects snapshot paths by @ and uses the volume-with-snapshot agent API only when COW snapshot size sync is required. Test: git diff --cached --check Test: mvn -pl header,storage -Dmaven.test.skip=true clean package Resolves: ZSTAC-67903 Change-Id: I0f4c2b8e3a9d167f5c0e2b6a1d9c8e7f3456abcd
MatheMatrix
force-pushed
the
sync/xiangheng.zhao/fix-zbs-snapshot-capacity-not-update@@3
branch
from
b0700e5 to
9f87754
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
1 participant
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.
ZBS COPY_ON_WRITE snapshot actual size is maintained by the storage backend and may change after snapshot creation. The console reads snapshot size from VolumeSnapshotVO, but the scheduled active-volume size sync refreshed only volume actual size before, so snapshot capacity could stay stale in the database.
During scheduled active-volume size sync, only primary storages whose snapshot mode is COPY_ON_WRITE attach snapshot install paths to the batch volume resource size sync request. The reply carries snapshot actual sizes so VolumeSnapshotVO.size can be refreshed together with volume actual size, without adding a manual snapshot size query flow for the console.
Rename BatchSyncVolumeSizeOnPrimaryStorageMsg to BatchSyncVolumeResourceSizeOnPrimaryStorageMsg because the scheduled batch sync now covers volume resources and, for COW snapshot backends, snapshot resources. Keep SyncVolumeSize and single-volume stats as volume-only flows.
Query snapshot capability by primary storage identity and volume type. PrimaryStorageInventory#getIdentity provides the cache identity: normal primary storages use type, and external primary storages override it with their identity, so VolumeManagerImpl does not depend on external storage VOs or external/non-external type checks.
Use StorageResourceStats as the batchStats return parent for VolumeStats and VolumeSnapshotStats. Batch stats accepts one installPaths collection; ZBS detects snapshot paths by @ and uses the volume-with-snapshot agent API only when COW snapshot size sync is required.
Test: git diff --cached --check
Test: mvn -pl header,storage -Dmaven.test.skip=true clean package
Resolves: ZSTAC-67903
Change-Id: I0f4c2b8e3a9d167f5c0e2b6a1d9c8e7f3456abcd
sync from gitlab !10408