Merge pull request #6061 from shunwang-crypto/fix/ops-mixing-cgroup-host-memory
fix(ops): avoid mixing cgroup and host memory metrics
This commit is contained in:
@@ -601,24 +601,11 @@ func (c *OpsMetricsCollector) collectSystemStats(ctx context.Context) (*opsColle
|
||||
|
||||
sampleAt := time.Now().UTC()
|
||||
|
||||
// Prefer cgroup (container) metrics when available.
|
||||
// CPU: prefer cgroup (container) metrics, falling back to host metrics when
|
||||
// cgroup CPU accounting is unavailable.
|
||||
if cpuPct := c.tryCgroupCPUPercent(sampleAt); cpuPct != nil {
|
||||
out.cpuUsagePercent = cpuPct
|
||||
}
|
||||
|
||||
cgroupUsed, cgroupTotal, cgroupOK := readCgroupMemoryBytes()
|
||||
if cgroupOK {
|
||||
usedMB := int64(cgroupUsed / bytesPerMB)
|
||||
out.memoryUsedMB = &usedMB
|
||||
if cgroupTotal > 0 {
|
||||
totalMB := int64(cgroupTotal / bytesPerMB)
|
||||
out.memoryTotalMB = &totalMB
|
||||
pct := roundTo1DP(float64(cgroupUsed) / float64(cgroupTotal) * 100)
|
||||
out.memoryUsagePercent = &pct
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback to host metrics if cgroup metrics are unavailable (or incomplete).
|
||||
if out.cpuUsagePercent == nil {
|
||||
if cpuPercents, err := cpu.PercentWithContext(ctx, 0, false); err == nil && len(cpuPercents) > 0 {
|
||||
v := roundTo1DP(cpuPercents[0])
|
||||
@@ -626,32 +613,60 @@ func (c *OpsMetricsCollector) collectSystemStats(ctx context.Context) (*opsColle
|
||||
}
|
||||
}
|
||||
|
||||
// If total memory isn't available from cgroup (e.g. memory.max = "max"), fill total from host.
|
||||
if out.memoryUsedMB == nil || out.memoryTotalMB == nil || out.memoryUsagePercent == nil {
|
||||
if vm, err := mem.VirtualMemoryWithContext(ctx); err == nil && vm != nil {
|
||||
if out.memoryUsedMB == nil {
|
||||
usedMB := int64(vm.Used / bytesPerMB)
|
||||
out.memoryUsedMB = &usedMB
|
||||
}
|
||||
if out.memoryTotalMB == nil {
|
||||
totalMB := int64(vm.Total / bytesPerMB)
|
||||
out.memoryTotalMB = &totalMB
|
||||
}
|
||||
if out.memoryUsagePercent == nil {
|
||||
if out.memoryUsedMB != nil && out.memoryTotalMB != nil && *out.memoryTotalMB > 0 {
|
||||
pct := roundTo1DP(float64(*out.memoryUsedMB) / float64(*out.memoryTotalMB) * 100)
|
||||
out.memoryUsagePercent = &pct
|
||||
} else {
|
||||
pct := roundTo1DP(vm.UsedPercent)
|
||||
out.memoryUsagePercent = &pct
|
||||
}
|
||||
}
|
||||
// Memory: prefer cgroup (container) metrics, but only when the cgroup exposes
|
||||
// BOTH a current usage and a concrete limit (memory.max != "max"). When the
|
||||
// limit is missing or the cgroup data is otherwise incomplete, fall back
|
||||
// ENTIRELY to host metrics. Never mix a container "used" with a host "total":
|
||||
// doing so reports a misleadingly tiny percentage (e.g. 60MB / 23GB ~= 0.3%).
|
||||
cgroupUsed, cgroupTotal, cgroupOK := readCgroupMemoryBytes()
|
||||
var host *mem.VirtualMemoryStat
|
||||
if !cgroupOK || cgroupTotal == 0 {
|
||||
if vm, err := mem.VirtualMemoryWithContext(ctx); err == nil {
|
||||
host = vm
|
||||
}
|
||||
}
|
||||
out.memoryUsedMB, out.memoryTotalMB, out.memoryUsagePercent = resolveMemoryStats(cgroupUsed, cgroupTotal, cgroupOK, host)
|
||||
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// resolveMemoryStats picks a single, self-consistent (used, total, percent)
|
||||
// memory trio from either cgroup (container) or host metrics — never a mix of
|
||||
// the two.
|
||||
//
|
||||
// cgroup metrics are preferred, but only when the cgroup reports BOTH a current
|
||||
// usage and a concrete limit (i.e. memory.max is a number, not "max", so
|
||||
// cgroupTotal > 0). If the limit is absent or the cgroup data is otherwise
|
||||
// incomplete, all three values fall back to the host reading. This avoids the
|
||||
// classic pitfall of dividing a container "used" by a host "total", which
|
||||
// wildly understates memory usage (e.g. 60MB / 23GB ~= 0.3%).
|
||||
func resolveMemoryStats(cgroupUsed, cgroupTotal uint64, cgroupOK bool, host *mem.VirtualMemoryStat) (usedMB *int64, totalMB *int64, usagePercent *float64) {
|
||||
if cgroupOK && cgroupTotal > 0 {
|
||||
u := int64(cgroupUsed / bytesPerMB)
|
||||
t := int64(cgroupTotal / bytesPerMB)
|
||||
p := roundTo1DP(float64(cgroupUsed) / float64(cgroupTotal) * 100)
|
||||
return &u, &t, &p
|
||||
}
|
||||
|
||||
if host == nil {
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
u := int64(host.Used / bytesPerMB)
|
||||
usedMB = &u
|
||||
if host.Total > 0 {
|
||||
t := int64(host.Total / bytesPerMB)
|
||||
totalMB = &t
|
||||
p := roundTo1DP(float64(host.Used) / float64(host.Total) * 100)
|
||||
usagePercent = &p
|
||||
} else {
|
||||
// Degenerate: host reported no total. Preserve gopsutil's own percentage.
|
||||
p := roundTo1DP(host.UsedPercent)
|
||||
usagePercent = &p
|
||||
}
|
||||
return usedMB, totalMB, usagePercent
|
||||
}
|
||||
|
||||
func (c *OpsMetricsCollector) tryCgroupCPUPercent(now time.Time) *float64 {
|
||||
usageNanos, ok := readCgroupCPUUsageNanos()
|
||||
if !ok {
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/shirou/gopsutil/v4/mem"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
const (
|
||||
testMiB = 1024 * 1024
|
||||
testGiB = 1024 * testMiB
|
||||
)
|
||||
|
||||
// TestResolveMemoryStatsCgroupUsageButUnlimitedFallsBackToHost is the core
|
||||
// regression: in Docker + cgroup v2 with NO memory limit, memory.current is a
|
||||
// small container number while memory.max = "max" (so cgroupTotal == 0). The
|
||||
// old code reported the container "used" against the host "total", producing a
|
||||
// misleadingly tiny percentage (~0.3%). The fix must fall back ENTIRELY to host
|
||||
// metrics instead of mixing the two sources.
|
||||
func TestResolveMemoryStatsCgroupUsageButUnlimitedFallsBackToHost(t *testing.T) {
|
||||
const cgroupUsed = uint64(64573440) // memory.current, ~61 MiB
|
||||
host := &mem.VirtualMemoryStat{
|
||||
Used: 16 * testGiB,
|
||||
Total: 24 * testGiB,
|
||||
UsedPercent: 66.7,
|
||||
}
|
||||
|
||||
usedMB, totalMB, pct := resolveMemoryStats(cgroupUsed, 0 /* memory.max = "max" */, true, host)
|
||||
|
||||
require.NotNil(t, usedMB)
|
||||
require.NotNil(t, totalMB)
|
||||
require.NotNil(t, pct)
|
||||
|
||||
// Everything must come from the host, not from the cgroup container value.
|
||||
require.Equal(t, int64(16*1024), *usedMB, "used must be host used, not container used")
|
||||
require.Equal(t, int64(24*1024), *totalMB, "total must be host total")
|
||||
require.InDelta(t, 66.7, *pct, 0.05, "percent must be host-derived, not container/host mix")
|
||||
|
||||
// Guard against the specific bug: container used (~61 MiB) vs host total (~0.3%).
|
||||
require.NotEqual(t, int64(cgroupUsed/testMiB), *usedMB, "must not report the container used value")
|
||||
require.Greater(t, *pct, 1.0, "percent must not collapse to the ~0.3%% mixed value")
|
||||
}
|
||||
|
||||
// TestResolveMemoryStatsExplicitContainerLimitUsesCgroup covers the case where
|
||||
// a real container memory limit is set: memory.current = 512 MiB and
|
||||
// memory.max = 2 GiB must yield ~25% entirely from cgroup, ignoring the host.
|
||||
func TestResolveMemoryStatsExplicitContainerLimitUsesCgroup(t *testing.T) {
|
||||
host := &mem.VirtualMemoryStat{
|
||||
Used: 16 * testGiB, // deliberately different; must be ignored
|
||||
Total: 24 * testGiB,
|
||||
UsedPercent: 66.7,
|
||||
}
|
||||
|
||||
usedMB, totalMB, pct := resolveMemoryStats(512*testMiB, 2*testGiB, true, host)
|
||||
|
||||
require.NotNil(t, usedMB)
|
||||
require.NotNil(t, totalMB)
|
||||
require.NotNil(t, pct)
|
||||
|
||||
require.Equal(t, int64(512), *usedMB)
|
||||
require.Equal(t, int64(2048), *totalMB)
|
||||
require.InDelta(t, 25.0, *pct, 0.05)
|
||||
}
|
||||
|
||||
// TestResolveMemoryStatsNoCgroupUsesHost covers bare-metal / no-cgroup hosts:
|
||||
// all three values come from the host reading.
|
||||
func TestResolveMemoryStatsNoCgroupUsesHost(t *testing.T) {
|
||||
host := &mem.VirtualMemoryStat{
|
||||
Used: 16 * testGiB,
|
||||
Total: 24 * testGiB,
|
||||
UsedPercent: 66.7,
|
||||
}
|
||||
|
||||
usedMB, totalMB, pct := resolveMemoryStats(0, 0, false, host)
|
||||
|
||||
require.NotNil(t, usedMB)
|
||||
require.NotNil(t, totalMB)
|
||||
require.NotNil(t, pct)
|
||||
require.Equal(t, int64(16*1024), *usedMB)
|
||||
require.Equal(t, int64(24*1024), *totalMB)
|
||||
require.InDelta(t, 66.7, *pct, 0.05)
|
||||
}
|
||||
|
||||
// TestResolveMemoryStatsNoDataReturnsNil: when neither cgroup nor host data is
|
||||
// available, all outputs are nil (best-effort, nothing persisted).
|
||||
func TestResolveMemoryStatsNoDataReturnsNil(t *testing.T) {
|
||||
usedMB, totalMB, pct := resolveMemoryStats(0, 0, false, nil)
|
||||
require.Nil(t, usedMB)
|
||||
require.Nil(t, totalMB)
|
||||
require.Nil(t, pct)
|
||||
}
|
||||
|
||||
// TestResolveMemoryStatsHostWithoutTotalKeepsGopsutilPercent: degenerate host
|
||||
// reading with no total still yields a used value and gopsutil's own percentage.
|
||||
func TestResolveMemoryStatsHostWithoutTotalKeepsGopsutilPercent(t *testing.T) {
|
||||
host := &mem.VirtualMemoryStat{
|
||||
Used: 8 * testGiB,
|
||||
Total: 0,
|
||||
UsedPercent: 42.5,
|
||||
}
|
||||
|
||||
usedMB, totalMB, pct := resolveMemoryStats(0, 0, false, host)
|
||||
|
||||
require.NotNil(t, usedMB)
|
||||
require.Nil(t, totalMB)
|
||||
require.NotNil(t, pct)
|
||||
require.Equal(t, int64(8*1024), *usedMB)
|
||||
require.InDelta(t, 42.5, *pct, 0.05)
|
||||
}
|
||||
Reference in New Issue
Block a user