Skip to content

[ZSTAC-85134] Carry cache pool capacity in KVM responses#3958

Open
zstack-robot-2 wants to merge 1 commit into
feature-5.5.6-local-cachefrom
sync/haidong.pang/codex/ZSTAC-85134-refresh-cache-pool-capacity@@3
Open

[ZSTAC-85134] Carry cache pool capacity in KVM responses#3958
zstack-robot-2 wants to merge 1 commit into
feature-5.5.6-local-cachefrom
sync/haidong.pang/codex/ZSTAC-85134-refresh-cache-pool-capacity@@3

Conversation

@zstack-robot-2
Copy link
Copy Markdown
Collaborator

为缓存池相关 KVM agent response 统一补充容量字段,使 PoolRsp、PoolHealthRsp、PoolCapacityRsp、GCPoolRsp 都能携带当前缓存池容量信息,供 MN 在业务响应中直接刷新容量。

sync from gitlab !9849

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Review Change Stack

Warning

Rate limit exceeded

@MatheMatrix has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 45 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 9f5dbd1a-5176-459c-b785-417ff2f6e533

📥 Commits

Reviewing files that changed from the base of the PR and between 87ad2f5 and 39d16f6.

📒 Files selected for processing (1)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java

Walkthrough

将池容量字段统一移入新基类 PoolRsp(新增 totalavailableallocateddirty),并将 PoolCapacityRspGCPoolRsp 改为继承 PoolRsp,移除子类中的重复容量字段(包括 used)。

变更内容

池响应类层级整合

Layer / File(s) Summary
PoolRsp 基类与子类继承调整
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
PoolRsp 增加容量字段 totalavailableallocateddirty(均为 Long)。PoolCapacityRspGCPoolRsp 的基类由 AgentResponse 改为 PoolRspPoolCapacityRsp 中原有的容量字段(包括 used)已移除,仅通过基类提供这些字段。

估计代码审查工作量

🎯 3 (Moderate) | ⏱️ ~20 minutes

兔子的诗

🐰 字段归位好整齐,
基类承载少重复,
继承一改更简洁,
代码轻快跳跃起,
小兔点头笑开怀。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed PR title follows the required format with JIRA key and clear description, staying within 72 character limit.
Description check ✅ Passed PR description explains the purpose of adding capacity fields to KVM response DTOs and relates to the changeset modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/haidong.pang/codex/ZSTAC-85134-refresh-cache-pool-capacity@@3

Comment @coderabbitai help to get the list of available commands and usage tips.

@zstack-robot-1
Copy link
Copy Markdown
Collaborator

Comment from yaohua.wu:

Review: MR !9849 — ZSTAC-85134

关联 MR

  • zstack-utility !7053 — KVM agent 在 Pool 类响应中填充新的容量字段
  • premium !13885 — HostCacheStoreBase 通过响应直接刷新 HostCacheStoreCapacityVO,并把显式同步纳入 ChainTask 队列

本 MR 是这套修复的"线协议基线":在 PoolRsp 上新增 total/used/available/allocated/dirty 五个字段,并把 PoolHealthRsp/PoolCapacityRsp/GCPoolRsp 改为 extends PoolRsp。下游(utility 的 schemas.py 和 premium 的 updateCapacityFromResponse)依赖这里的字段名与类型一致。

跨仓一致性核查

字段 zstack (Java) zstack-utility (Python) premium 读取
total Long int | None poolRsp.total
available Long int | None poolRsp.available
allocated Long int | None poolRsp.allocated(带 null 守护)✓
dirty Long int | None poolRsp.dirty(带 null 守护)✓
used Long int | None 不读取(见 Suggestion 2)

PoolCapacityRsp 重构后变成空类(字段全部下沉到 PoolRsp)。由于 JSON 序列化看的是字段名,这与旧 PoolCapacityRsp 直接声明这些字段在线格式上完全等价 — 旧管理节点 ↔ 新 agent、新管理节点 ↔ 旧 agent 都能正确反序列化已有字段,线兼容性 OK

PoolHealthRsp/GCPoolRsp 现在通过继承额外携带 5 个容量字段;旧版客户端反序列化时会忽略未知字段,前向兼容 OK。premium 侧 grayUpgrade.json 已经把新字段统一标注为 5.5.18,符合 ZStack 灰度升级框架要求。

Findings

🟢 Suggestion — 文档化 PoolHealthRsp 的语义扩展

PoolHealthRsp 原本是"聚合健康信号",现在因继承 PoolRsp 自动携带容量。配合 utility 端 _to_pool_health_rsp 在 check_pool 路径里调用 _fill_pool_rsp,这意味着每次健康检查都会回传容量并落库(见 premium 侧 updateCapacityFromResponse 在通用 httpCall 成功路径触发)。这是个语义扩展:PoolHealthRsp 不再只是"健康"。

建议:要么在类上加一行注释说明 health response 也会携带最新容量;要么如果你觉得健康通道不该承担容量同步职责,则在 _to_pool_health_rsp 不调用 _fill_pool_rsp、保持 PoolHealthRsp 容量字段为 null(premium 侧已经按 null 做了短路)。

Coverage

  • 单文件 1 个 hunk,纯线协议变更;本身没有运行时分支
  • 验证策略集中在 utility !7053 + premium !13885 的回归用例
  • Jira ZSTAC-85134 描述的根因(扩容后容量未即时刷新)不在本 MR 直接修,但本 MR 是修复的前提

Verdict: APPROVED

线协议变更最小、对称、向下兼容。无阻塞性问题。


🤖 Robot Reviewer

@MatheMatrix MatheMatrix force-pushed the sync/haidong.pang/codex/ZSTAC-85134-refresh-cache-pool-capacity@@3 branch from 738bef5 to 87ad2f5 Compare May 14, 2026 07:04
@MatheMatrix
Copy link
Copy Markdown
Owner

Comment from haidong.pang:

已调整:PoolHealthRsp 不再继承 PoolRsp,check_pool 响应不携带容量,容量只放在会改变缓存池状态的 pool 操作响应里。used 字段也已经从协议中移除。

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 5272-5284: The PoolRsp model in KVMAgentCommands is missing the
'used' capacity field and PoolHealthRsp should extend PoolRsp so health
responses reuse the same capacity carrier; update the PoolRsp class to add a
public Long used; then change PoolHealthRsp to extend PoolRsp (retain its public
Boolean healthy and public String reason fields) so health responses include
poolUuid/mountPoint/total/available/allocated/dirty/used; make the analogous
changes to the other duplicate definitions mentioned (the other
PoolRsp/PoolHealthRsp occurrences).
🪄 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: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 2e9f4772-c784-4d32-9c4b-76a367334d1f

📥 Commits

Reviewing files that changed from the base of the PR and between 738bef5 and 87ad2f5.

📒 Files selected for processing (1)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java

Comment on lines 5272 to 5284
public static class PoolRsp extends AgentResponse {
public String poolUuid;
public String mountPoint;
public Long total;
public Long available;
public Long allocated;
public Long dirty;
}

public static class PoolHealthRsp extends AgentResponse {
public Boolean healthy;
public String reason;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

缓存池响应模型未完整对齐容量协议(缺 usedPoolHealthRsp 未继承 PoolRsp)。

当前实现会导致容量字段集合不完整,并且健康响应无法复用同一容量载体;这与本次“统一从响应刷新容量”的目标不一致。

建议修改
 public static class PoolRsp extends AgentResponse {
     public String poolUuid;
     public String mountPoint;
     public Long total;
+    public Long used;
     public Long available;
     public Long allocated;
     public Long dirty;
 }

-public static class PoolHealthRsp extends AgentResponse {
+public static class PoolHealthRsp extends PoolRsp {
     public Boolean healthy;
     public String reason;
 }

Also applies to: 5286-5287, 5295-5298

🤖 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/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines
5272 - 5284, The PoolRsp model in KVMAgentCommands is missing the 'used'
capacity field and PoolHealthRsp should extend PoolRsp so health responses reuse
the same capacity carrier; update the PoolRsp class to add a public Long used;
then change PoolHealthRsp to extend PoolRsp (retain its public Boolean healthy
and public String reason fields) so health responses include
poolUuid/mountPoint/total/available/allocated/dirty/used; make the analogous
changes to the other duplicate definitions mentioned (the other
PoolRsp/PoolHealthRsp occurrences).

Add cache pool capacity fields to mutating pool-related KVM agent response DTOs so MN can refresh capacity from operation responses.

Resolves: ZSTAC-85134

Change-Id: I04b8c898a8fcc213f025a4db848e2f4cb97689f5
@MatheMatrix MatheMatrix force-pushed the sync/haidong.pang/codex/ZSTAC-85134-refresh-cache-pool-capacity@@3 branch from 87ad2f5 to 39d16f6 Compare May 14, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants