fix(metrics): correct cpu/disk% chart scaling, evict stale collector state, lighten nic query, drop polling & dead RRD code
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -21,6 +21,7 @@ from ...ipam_service import IPAMService
|
||||
from ._helpers import (check_vm_access, require_vm_access, _get_proxmox_client,
|
||||
get_next_vmid, archive_and_delete_snapshots,
|
||||
save_vm_instance, get_vm_instance, soft_delete_vm_instance)
|
||||
from ...services.metrics_history import query_instance_metrics, timeframe_to_range
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
@@ -1848,7 +1849,6 @@ def get_container_rrddata(
|
||||
# ==================== Metrics History (DB-only) ====================
|
||||
|
||||
def _resolve_metrics_window(timeframe, from_ts, to_ts):
|
||||
from ...services.metrics_history import timeframe_to_range
|
||||
from ...config import settings
|
||||
now = int(time_lib.time())
|
||||
if from_ts is not None and to_ts is not None:
|
||||
@@ -1874,7 +1874,6 @@ def get_vm_metrics(
|
||||
current_user: User = Depends(PermissionChecker("vm:view")),
|
||||
):
|
||||
"""Retrieve VM metrics time-series from the local DB (no Proxmox call)."""
|
||||
from ...services.metrics_history import query_instance_metrics
|
||||
f, t = _resolve_metrics_window(timeframe, from_ts, to_ts)
|
||||
return query_instance_metrics(db, server_id, vmid, f, t, nic)
|
||||
|
||||
@@ -1892,7 +1891,6 @@ def get_container_metrics(
|
||||
current_user: User = Depends(PermissionChecker("vm:view")),
|
||||
):
|
||||
"""Retrieve container metrics time-series from the local DB (no Proxmox call)."""
|
||||
from ...services.metrics_history import query_instance_metrics
|
||||
f, t = _resolve_metrics_window(timeframe, from_ts, to_ts)
|
||||
return query_instance_metrics(db, server_id, vmid, f, t, nic)
|
||||
|
||||
|
||||
@@ -26,6 +26,8 @@ def collect_once(db, items, clients, prev, _now=None):
|
||||
|
||||
# netstat is one call per (server_id, node); cache per tick.
|
||||
netstat_cache: dict[tuple, list] = {}
|
||||
# Track which prev keys are touched this tick so stale entries can be evicted.
|
||||
seen: set = set()
|
||||
|
||||
for it in items:
|
||||
if it.get("status") != "running":
|
||||
@@ -76,6 +78,7 @@ def collect_once(db, items, clients, prev, _now=None):
|
||||
iops_write=rate("wr_ops") if inst_type == "qemu" else None,
|
||||
))
|
||||
prev[key] = (now, cur)
|
||||
seen.add(key)
|
||||
|
||||
# per-NIC
|
||||
ck = (sid, node)
|
||||
@@ -93,9 +96,15 @@ def collect_once(db, items, clients, prev, _now=None):
|
||||
out_rate=compute_rate(npc.get("out"), float(dout), ndt),
|
||||
))
|
||||
prev[nkey] = (now, {"in": float(din), "out": float(dout)})
|
||||
seen.add(nkey)
|
||||
except Exception as e:
|
||||
logger.debug(f"[metrics_collector] {sid}:{vmid} error: {e}")
|
||||
|
||||
# Evict stale keys from prev (VMs/NICs not seen this tick).
|
||||
for k in list(prev):
|
||||
if k not in seen:
|
||||
del prev[k]
|
||||
|
||||
|
||||
async def metrics_collector_loop(db_factory) -> None:
|
||||
from ..config import settings
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from datetime import timezone as _tz
|
||||
|
||||
_RD_OPS = re.compile(r"rd_operations=(\d+)")
|
||||
_WR_OPS = re.compile(r"wr_operations=(\d+)")
|
||||
@@ -93,8 +94,6 @@ def aggregate_series(rows, from_ts, bucket, fields):
|
||||
return out
|
||||
|
||||
|
||||
from datetime import timezone as _tz
|
||||
|
||||
_AGG_FIELDS = ["cpu", "mem", "maxmem", "netin", "netout",
|
||||
"diskread", "diskwrite", "diskpct", "iops_read", "iops_write"]
|
||||
|
||||
@@ -119,16 +118,24 @@ def query_instance_metrics(db, server_id, vmid, from_ts, to_ts, nic="all"):
|
||||
.order_by(InstanceMetric.ts.asc()).all())
|
||||
|
||||
# Distinct NICs seen in the window (for the selector).
|
||||
nic_rows = (db.query(InstanceNicMetric)
|
||||
.filter(InstanceNicMetric.server_id == server_id,
|
||||
InstanceNicMetric.vmid == vmid,
|
||||
InstanceNicMetric.ts >= start, InstanceNicMetric.ts <= end)
|
||||
.order_by(InstanceNicMetric.ts.asc()).all())
|
||||
nics = sorted({r.dev for r in nic_rows})
|
||||
|
||||
# Optional per-NIC override of net rates, keyed by exact timestamp.
|
||||
nic_by_ts: dict[int, tuple] = {}
|
||||
if nic != "all":
|
||||
if nic == "all":
|
||||
# Only fetch distinct dev names — avoids loading all NIC rows.
|
||||
dev_rows = (db.query(InstanceNicMetric.dev)
|
||||
.filter(InstanceNicMetric.server_id == server_id,
|
||||
InstanceNicMetric.vmid == vmid,
|
||||
InstanceNicMetric.ts >= start, InstanceNicMetric.ts <= end)
|
||||
.distinct().all())
|
||||
nics = sorted({r.dev for r in dev_rows})
|
||||
nic_by_ts: dict[int, tuple] = {}
|
||||
else:
|
||||
# Load full NIC rows for the requested dev to build per-ts override map.
|
||||
nic_rows = (db.query(InstanceNicMetric)
|
||||
.filter(InstanceNicMetric.server_id == server_id,
|
||||
InstanceNicMetric.vmid == vmid,
|
||||
InstanceNicMetric.ts >= start, InstanceNicMetric.ts <= end)
|
||||
.order_by(InstanceNicMetric.ts.asc()).all())
|
||||
nics = sorted({r.dev for r in nic_rows})
|
||||
nic_by_ts = {}
|
||||
for r in nic_rows:
|
||||
if r.dev == nic:
|
||||
nic_by_ts[_to_ts(r.ts)] = (r.in_rate, r.out_rate)
|
||||
|
||||
@@ -49,3 +49,35 @@ def test_collect_once_second_tick_computes_rates(db_session):
|
||||
assert rows[1].iops_read == 5.0 # (110-100)/2
|
||||
nic = db_session.query(InstanceNicMetric).order_by(InstanceNicMetric.id).all()[-1]
|
||||
assert nic.in_rate == 10.0 # (1020-1000)/2
|
||||
|
||||
|
||||
def test_collect_once_evicts_stale_prev(db_session):
|
||||
"""Keys for VMs absent from the current tick must be pruned from prev."""
|
||||
def _client_for(vmid_str):
|
||||
c = MagicMock()
|
||||
c.get_vm_status.return_value = {
|
||||
"cpu": 0.1, "mem": 50, "maxmem": 100,
|
||||
"netin": 0, "netout": 0, "diskread": 0, "diskwrite": 0,
|
||||
}
|
||||
c.get_vm_fsinfo.return_value = [{"used": 5, "total": 20}]
|
||||
c.get_vm_blockstats.return_value = ""
|
||||
c.get_node_netstat.return_value = [
|
||||
{"dev": f"tap{vmid_str}i0", "vmid": vmid_str, "in": "0", "out": "0"}
|
||||
]
|
||||
return c
|
||||
|
||||
prev = {}
|
||||
# Tick 1: only vmid 114 running
|
||||
items_114 = [{"server_id": 1, "vmid": 114, "type": "qemu", "node": "prod", "status": "running"}]
|
||||
clients_114 = {1: _client_for("114")}
|
||||
collect_once(db_session, items_114, clients_114, prev, _now=100.0)
|
||||
# After tick 1, prev must contain 114 keys
|
||||
assert any("114" in k for k in prev), "Expected 114 keys in prev after tick 1"
|
||||
|
||||
# Tick 2: only vmid 115 running (114 is gone)
|
||||
items_115 = [{"server_id": 1, "vmid": 115, "type": "qemu", "node": "prod", "status": "running"}]
|
||||
clients_115 = {1: _client_for("115")}
|
||||
collect_once(db_session, items_115, clients_115, prev, _now=102.0)
|
||||
# 114 keys must be evicted; 115 keys must be present
|
||||
assert not any("114" in k for k in prev), "Expected 114 keys evicted from prev after tick 2"
|
||||
assert any("115" in k for k in prev), "Expected 115 keys in prev after tick 2"
|
||||
|
||||
@@ -235,7 +235,7 @@ export default function GraphsTab({ serverId, vmid, type, node }: Props) {
|
||||
data={points}
|
||||
dataKey="cpu"
|
||||
color="hsl(217, 91%, 60%)"
|
||||
formatValue={(v) => `${(v * 100).toFixed(1)}%`}
|
||||
formatValue={(v) => `${v.toFixed(1)}%`}
|
||||
unit="%"
|
||||
/>
|
||||
<ChartCard
|
||||
@@ -280,7 +280,7 @@ export default function GraphsTab({ serverId, vmid, type, node }: Props) {
|
||||
data={points}
|
||||
dataKey="diskpct"
|
||||
color="hsl(45, 93%, 47%)"
|
||||
formatValue={(v) => `${(v * 100).toFixed(1)}%`}
|
||||
formatValue={(v) => `${v.toFixed(1)}%`}
|
||||
unit="%"
|
||||
/>
|
||||
<ChartCard
|
||||
|
||||
@@ -12,7 +12,6 @@ export const vmKeys = {
|
||||
detail: (serverId: number, vmid: number) => ['vm', serverId, vmid] as const,
|
||||
config: (serverId: number, vmid: number) => ['vm', serverId, vmid, 'config'] as const,
|
||||
status: (serverId: number, vmid: number) => ['vm', serverId, vmid, 'status'] as const,
|
||||
rrddata: (serverId: number, vmid: number, tf: string) => ['vm', serverId, vmid, 'rrddata', tf] as const,
|
||||
snapshots: (serverId: number, vmid: number) => ['vm', serverId, vmid, 'snapshots'] as const,
|
||||
interfaces: (serverId: number, vmid: number) => ['vm', serverId, vmid, 'interfaces'] as const,
|
||||
};
|
||||
@@ -178,28 +177,6 @@ export function useVMConfig(serverId: number, vmid: number, type: string, node:
|
||||
});
|
||||
}
|
||||
|
||||
// RRD refresh cadence per timeframe — matches Proxmox's native RRA resolution,
|
||||
// so the chart advances as new samples land instead of staying frozen.
|
||||
const RRD_REFETCH_MS: Record<string, number> = {
|
||||
hour: 60_000, // 1-min resolution
|
||||
day: 300_000, // ~5-min
|
||||
week: 1_800_000, // ~30-min
|
||||
month: 3_600_000, // ~hourly
|
||||
year: 3_600_000,
|
||||
};
|
||||
|
||||
export function useVMRrddata(serverId: number, vmid: number, type: string, node: string, timeframe = 'hour', enabled = true) {
|
||||
const prefix = type === 'lxc' ? 'container' : 'vm';
|
||||
return useQuery({
|
||||
queryKey: vmKeys.rrddata(serverId, vmid, timeframe),
|
||||
queryFn: async () => {
|
||||
const res = await apiClient.get<{ data: unknown[]; timeframe: string }>(`/proxmox/api/${serverId}/${prefix}/${vmid}/rrddata?node=${node}&timeframe=${timeframe}`);
|
||||
return res.data ?? [];
|
||||
},
|
||||
enabled,
|
||||
refetchInterval: enabled ? (RRD_REFETCH_MS[timeframe] ?? 60_000) : false,
|
||||
});
|
||||
}
|
||||
|
||||
export interface MetricPoint {
|
||||
time: number;
|
||||
@@ -230,7 +207,6 @@ export function useVMMetrics(
|
||||
}
|
||||
return apiClient.get(`/proxmox/api/${serverId}/${prefix}/${vmid}/metrics?${qs}`);
|
||||
},
|
||||
refetchInterval: 30000,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user