Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zstack-robot-2 wants to merge 1 commit into
base: feature-5.5.6-local-cache
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions conf/db/upgrade/V5.5.18__schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ CREATE TABLE IF NOT EXISTS `HostCacheStoreCapacityVO` (
`uuid` VARCHAR(32) NOT NULL,
`totalCapacity` BIGINT NOT NULL DEFAULT 0,
`availableCapacity` BIGINT NOT NULL DEFAULT 0,
`allocated` BIGINT NOT NULL DEFAULT 0,
`dirty` BIGINT NOT NULL DEFAULT 0,
`totalPhysicalCapacity` BIGINT NOT NULL DEFAULT 0,
`availablePhysicalCapacity` BIGINT NOT NULL DEFAULT 0,
`systemUsedCapacity` BIGINT NOT NULL DEFAULT 0,
Comment on lines +22 to +24
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 18, 2026 •
edited
Loading

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

新增列名需使用反引号包裹，且升级场景未正确处理。

根据编码规范，所有列名必须使用反引号包裹以避免 MySQL 8.0 / GreatSQL 保留关键字冲突。此外，CREATE TABLE IF NOT EXISTS 语句在表已存在时不会添加新列，升级场景需要使用 ALTER TABLE 或存储过程来处理。

建议修改：添加 ALTER TABLE 语句处理升级场景 
 CREATE TABLE IF NOT EXISTS `HostCacheStoreCapacityVO` (
     `uuid`              VARCHAR(32)    NOT NULL,
     `totalCapacity`     BIGINT         NOT NULL DEFAULT 0,
     `availableCapacity` BIGINT         NOT NULL DEFAULT 0,
-    `totalPhysicalCapacity`     BIGINT NOT NULL DEFAULT 0,
-    `availablePhysicalCapacity` BIGINT NOT NULL DEFAULT 0,
-    `systemUsedCapacity`        BIGINT NOT NULL DEFAULT 0,
+    `totalPhysicalCapacity`     BIGINT NOT NULL DEFAULT 0,
+    `availablePhysicalCapacity` BIGINT NOT NULL DEFAULT 0,
+    `systemUsedCapacity`        BIGINT NOT NULL DEFAULT 0,
     PRIMARY KEY (`uuid`),
     ...
 ) ENGINE = InnoDB DEFAULT CHARSET = utf8;

+-- Handle upgrade scenario: add new columns if table already exists
+CALL AddColumnIfNotExists('zstack', 'HostCacheStoreCapacityVO', 'totalPhysicalCapacity', 'BIGINT NOT NULL DEFAULT 0');
+CALL AddColumnIfNotExists('zstack', 'HostCacheStoreCapacityVO', 'availablePhysicalCapacity', 'BIGINT NOT NULL DEFAULT 0');
+CALL AddColumnIfNotExists('zstack', 'HostCacheStoreCapacityVO', 'systemUsedCapacity', 'BIGINT NOT NULL DEFAULT 0');

As per coding guidelines: "所有表名和列名必须使用反引号包裹" 以及 "When NOT NULL exists, must use stored procedure or other functions to process historical data"。

🤖 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/db/upgrade/V5.5.18__schema.sql` around lines 22 - 24, The new column
definitions use unquoted identifiers and are only inside a CREATE TABLE IF NOT
EXISTS block, which won't add columns for existing tables and may conflict with
reserved words; update the migration to wrap each column name in backticks
(e.g., `totalPhysicalCapacity`, `availablePhysicalCapacity`,
`systemUsedCapacity`) and add an ALTER TABLE path for upgrades (use ALTER TABLE
... ADD COLUMN ... with backticks) or implement a stored procedure that checks
for column existence and adds the NOT NULL columns while backfilling historical
rows safely (e.g., add with DEFAULT or add nullable then UPDATE existing rows
then ALTER to NOT NULL), referencing the CREATE TABLE IF NOT EXISTS, ALTER TABLE
and stored procedure approach in the migration.

Copy link
Copy Markdown
Collaborator Author

@zstack-robot-2 zstack-robot-2 May 18, 2026

Choose a reason for hiding this comment

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

Comment from haidong.pang:

未发布功能，不考虑该场景

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

@zstack-robot-2，明白了，既然 HostCacheStoreCapacityVO 是尚未发布的新表，不存在需要升级的历史数据，CREATE TABLE IF NOT EXISTS 方式完全可行，无需额外的 ALTER TABLE 升级路径。感谢说明！

(＾・ω・＾ )

🧠 Learnings used 
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 3416
File: conf/db/upgrade/V5.5.12__schema.sql:24-35
Timestamp: 2026-03-05T02:13:23.339Z
Learning: In ZStack, the database schema name is fixed as 'zstack'. For SQL upgrade scripts under conf/db/upgrade, continue using TABLE_SCHEMA = 'zstack' and reference tables as zstack.`UsedIpVO` (i.e., qualify with the schema). Do not replace with DATABASE() or remove the schema qualifier for portability. This pattern can be assumed across upgrade scripts in this directory.

Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 3891
File: conf/db/upgrade/V5.5.22__schema.sql:28-31
Timestamp: 2026-05-07T09:44:01.429Z
Learning: In ZStack MySQL upgrade scripts under conf/db/upgrade/*.sql, if a table already contains a TIMESTAMP column defined with ON UPDATE CURRENT_TIMESTAMP (e.g., lastOpDate), then any second TIMESTAMP column (e.g., createDate) must NOT use DEFAULT CURRENT_TIMESTAMP. This repository’s CI/validated environment uses MySQL 5.7 with explicit_defaults_for_timestamp=OFF, which can trigger MySQL Error 1293 (only one TIMESTAMP column with CURRENT_TIMESTAMP in DEFAULT/ON UPDATE). Use a non-CURRENT_TIMESTAMP sentinel default such as '2000-01-01 00:00:00' (and typically keep it NOT NULL), and rely on ModelServiceRefVO.PrePersist in Java to set the real creation timestamp for new JPA-managed rows.

PRIMARY KEY (`uuid`),
CONSTRAINT `fkHostCacheStoreCapacityVOHostCacheStoreVO`
FOREIGN KEY (`uuid`) REFERENCES `HostCacheStoreVO` (`uuid`)
Expand Down
34 changes: 22 additions & 12 deletions header/src/main/java/org/zstack/header/volumeCache/HostCacheStoreCapacityInventory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ public class HostCacheStoreCapacityInventory implements Serializable {
private String uuid;
private long totalCapacity;
private long availableCapacity;
private long allocated;
private long dirty;
private long totalPhysicalCapacity;
private long availablePhysicalCapacity;
private long systemUsedCapacity;

public static HostCacheStoreCapacityInventory valueOf(HostCacheStoreCapacityVO vo) {
if (vo == null) {
Expand All @@ -26,8 +27,9 @@ public static HostCacheStoreCapacityInventory valueOf(HostCacheStoreCapacityVO v
inv.setUuid(vo.getUuid());
inv.setTotalCapacity(vo.getTotalCapacity());
inv.setAvailableCapacity(vo.getAvailableCapacity());
inv.setAllocated(vo.getAllocated());
inv.setDirty(vo.getDirty());
inv.setTotalPhysicalCapacity(vo.getTotalPhysicalCapacity());
inv.setAvailablePhysicalCapacity(vo.getAvailablePhysicalCapacity());
inv.setSystemUsedCapacity(vo.getSystemUsedCapacity());
return inv;
}

Expand Down Expand Up @@ -67,19 +69,27 @@ public void setAvailableCapacity(long availableCapacity) {
this.availableCapacity = availableCapacity;
}

public long getAllocated() {
return allocated;
public long getTotalPhysicalCapacity() {
return totalPhysicalCapacity;
}

public void setAllocated(long allocated) {
this.allocated = allocated;
public void setTotalPhysicalCapacity(long totalPhysicalCapacity) {
this.totalPhysicalCapacity = totalPhysicalCapacity;
}

public long getDirty() {
return dirty;
public long getAvailablePhysicalCapacity() {
return availablePhysicalCapacity;
}

public void setDirty(long dirty) {
this.dirty = dirty;
public void setAvailablePhysicalCapacity(long availablePhysicalCapacity) {
this.availablePhysicalCapacity = availablePhysicalCapacity;
}

public long getSystemUsedCapacity() {
return systemUsedCapacity;
}

public void setSystemUsedCapacity(long systemUsedCapacity) {
this.systemUsedCapacity = systemUsedCapacity;
}
}
31 changes: 21 additions & 10 deletions header/src/main/java/org/zstack/header/volumeCache/HostCacheStoreCapacityVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ public class HostCacheStoreCapacityVO {
private long availableCapacity;

@Column
private long allocated;
private long totalPhysicalCapacity;

@Column
private long dirty;
private long availablePhysicalCapacity;

@Column
private long systemUsedCapacity;

public HostCacheStoreCapacityVO() {
}
Expand Down Expand Up @@ -61,19 +64,27 @@ public void setAvailableCapacity(long availableCapacity) {
this.availableCapacity = availableCapacity;
}

public long getAllocated() {
return allocated;
public long getTotalPhysicalCapacity() {
return totalPhysicalCapacity;
}

public void setTotalPhysicalCapacity(long totalPhysicalCapacity) {
this.totalPhysicalCapacity = totalPhysicalCapacity;
}

public long getAvailablePhysicalCapacity() {
return availablePhysicalCapacity;
}

public void setAllocated(long allocated) {
this.allocated = allocated;
public void setAvailablePhysicalCapacity(long availablePhysicalCapacity) {
this.availablePhysicalCapacity = availablePhysicalCapacity;
}

public long getDirty() {
return dirty;
public long getSystemUsedCapacity() {
return systemUsedCapacity;
}

public void setDirty(long dirty) {
this.dirty = dirty;
public void setSystemUsedCapacity(long systemUsedCapacity) {
this.systemUsedCapacity = systemUsedCapacity;
}
}
5 changes: 3 additions & 2 deletions header/src/main/java/org/zstack/header/volumeCache/HostCacheStoreCapacityVO_.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public class HostCacheStoreCapacityVO_ {
public static volatile SingularAttribute<HostCacheStoreCapacityVO, String> uuid;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> totalCapacity;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> availableCapacity;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> allocated;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> dirty;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> totalPhysicalCapacity;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> availablePhysicalCapacity;
public static volatile SingularAttribute<HostCacheStoreCapacityVO, Long> systemUsedCapacity;
}
42 changes: 27 additions & 15 deletions header/src/main/java/org/zstack/header/volumeCache/HostCacheStoreInventory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@ public class HostCacheStoreInventory implements Serializable {
private long availableCapacity;

@Queryable(mappingClass = HostCacheStoreCapacityInventory.class,
joinColumn = @JoinColumn(name = "uuid", referencedColumnName = "allocated"))
private long allocated;
joinColumn = @JoinColumn(name = "uuid", referencedColumnName = "totalPhysicalCapacity"))
private long totalPhysicalCapacity;

@Queryable(mappingClass = HostCacheStoreCapacityInventory.class,
joinColumn = @JoinColumn(name = "uuid", referencedColumnName = "dirty"))
private long dirty;
joinColumn = @JoinColumn(name = "uuid", referencedColumnName = "availablePhysicalCapacity"))
private long availablePhysicalCapacity;

@Queryable(mappingClass = HostCacheStoreCapacityInventory.class,
joinColumn = @JoinColumn(name = "uuid", referencedColumnName = "systemUsedCapacity"))
private long systemUsedCapacity;

private HostCacheStoreState state;
private HostCacheStoreStatus status;
Expand All @@ -52,8 +56,9 @@ public static HostCacheStoreInventory valueOf(HostCacheStoreVO vo) {
if (vo.getCapacity() != null) {
inv.setTotalCapacity(vo.getCapacity().getTotalCapacity());
inv.setAvailableCapacity(vo.getCapacity().getAvailableCapacity());
inv.setAllocated(vo.getCapacity().getAllocated());
inv.setDirty(vo.getCapacity().getDirty());
inv.setTotalPhysicalCapacity(vo.getCapacity().getTotalPhysicalCapacity());
inv.setAvailablePhysicalCapacity(vo.getCapacity().getAvailablePhysicalCapacity());
inv.setSystemUsedCapacity(vo.getCapacity().getSystemUsedCapacity());
}
inv.setState(vo.getState());
inv.setStatus(vo.getStatus());
Expand Down Expand Up @@ -143,20 +148,28 @@ public void setAvailableCapacity(long availableCapacity) {
this.availableCapacity = availableCapacity;
}

public long getAllocated() {
return allocated;
public long getTotalPhysicalCapacity() {
return totalPhysicalCapacity;
}

public void setTotalPhysicalCapacity(long totalPhysicalCapacity) {
this.totalPhysicalCapacity = totalPhysicalCapacity;
}

public void setAllocated(long allocated) {
this.allocated = allocated;
public long getAvailablePhysicalCapacity() {
return availablePhysicalCapacity;
}

public long getDirty() {
return dirty;
public void setAvailablePhysicalCapacity(long availablePhysicalCapacity) {
this.availablePhysicalCapacity = availablePhysicalCapacity;
}

public void setDirty(long dirty) {
this.dirty = dirty;
public long getSystemUsedCapacity() {
return systemUsedCapacity;
}

public void setSystemUsedCapacity(long systemUsedCapacity) {
this.systemUsedCapacity = systemUsedCapacity;
}

public HostCacheStoreState getState() {
Expand Down Expand Up @@ -207,4 +220,3 @@ public void setLastOpDate(Timestamp lastOpDate) {
this.lastOpDate = lastOpDate;
}
}

18 changes: 18 additions & 0 deletions header/src/main/java/org/zstack/header/volumeCache/HostCacheStoreInventoryDoc_zh_cn.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@ doc {
type "long"
since "5.5.6"
}
field {
name "totalPhysicalCapacity"
desc "主机缓存存储的物理总容量，单位为字节"
type "long"
since "5.5.6"
}
field {
name "availablePhysicalCapacity"
desc "主机缓存存储的物理可用容量，单位为字节"
type "long"
since "5.5.6"
}
field {
name "systemUsedCapacity"
desc "主机缓存存储中非缓存文件占用的容量，单位为字节"
type "long"
since "5.5.6"
}
ref {
name "state"
path "org.zstack.header.volumeCache.HostCacheStoreInventory.state"
Expand Down
16 changes: 9 additions & 7 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5272,19 +5272,21 @@ public static class DetachVolumeCacheCmd extends AgentCommand implements HasThre
public static class PoolRsp extends AgentResponse {
public String poolUuid;
public String mountPoint;
public Long totalCapacity;
public Long availableCapacity;
public Long totalPhysicalCapacity;
public Long availablePhysicalCapacity;
public Long systemUsedCapacity;
}

public static class PoolHealthRsp extends AgentResponse {
public String poolUuid;
public String mountPoint;
public Boolean healthy;
public String reason;
}
Comment on lines 5272 to 5287
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 14, 2026

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

缓存池响应模型未完整对齐容量协议（缺 used 且 PoolHealthRsp 未继承 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).


public static class PoolCapacityRsp extends AgentResponse {
public Long total;
public Long used;
public Long available;
public Long allocated;
public Long dirty;
public static class PoolCapacityRsp extends PoolRsp {
}

public static class CacheRsp extends AgentResponse {
Expand All @@ -5293,7 +5295,7 @@ public static class CacheRsp extends AgentResponse {
public Long actualSize;
}

public static class GCPoolRsp extends AgentResponse {
public static class GCPoolRsp extends PoolRsp {
public List<String> gcFiles;
public Integer gcCount;
}
Expand Down
28 changes: 18 additions & 10 deletions sdk/src/main/java/org/zstack/sdk/HostCacheStoreInventory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,28 @@ public long getAvailableCapacity() {
return this.availableCapacity;
}

public long allocated;
public void setAllocated(long allocated) {
this.allocated = allocated;
public long totalPhysicalCapacity;
public void setTotalPhysicalCapacity(long totalPhysicalCapacity) {
this.totalPhysicalCapacity = totalPhysicalCapacity;
}
public long getAllocated() {
return this.allocated;
public long getTotalPhysicalCapacity() {
return this.totalPhysicalCapacity;
}

public long dirty;
public void setDirty(long dirty) {
this.dirty = dirty;
public long availablePhysicalCapacity;
public void setAvailablePhysicalCapacity(long availablePhysicalCapacity) {
this.availablePhysicalCapacity = availablePhysicalCapacity;
}
public long getDirty() {
return this.dirty;
public long getAvailablePhysicalCapacity() {
return this.availablePhysicalCapacity;
}

public long systemUsedCapacity;
public void setSystemUsedCapacity(long systemUsedCapacity) {
this.systemUsedCapacity = systemUsedCapacity;
}
public long getSystemUsedCapacity() {
return this.systemUsedCapacity;
}

public HostCacheStoreState state;
Expand Down