From dc22d837448587a726dba4e69a4c818cb8674522 Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Tue, 1 Sep 2026 21:46:29 +0500 Subject: [PATCH] fix(slab): the guard watches memcg-attributed truth, not the drifting counter Diagnosed live on runner-2: /proc/meminfo SUnreclaim said 1041 MiB while the root cgroup's memory.stat attributed 44 MiB, and the ~1 GiB gap was unattributable to any live cache -- per-cache sums reached ~220 MiB -- and immune to drop_caches=2 (VFS caches fell 486 MiB, the counter did not move) and to per-cache shrink. The global NR_SLAB_UNRECLAIMABLE counter drifts under container churn on 6.8.0-138; the memory it reports is not occupied. The healer was rebooting members daily to reset an integer, and the alert paged on the same phantom. The compliance observer now exports both views side by side (slab_unreclaimable_counter_bytes / _attributed_bytes), slab-heal prefers the attributed value when cgroup v2 exposes it and names its source in the heal reason, and kernel_slab_unreclaimable watches the attributed gauge. The reboot path stays armed for genuine slab growth. Closes #264 Claude-Session: https://claude.ai/code/session_01LsGid6U5RrQdFvJmvYdGCF --- cmd/gha-fleet/main.go | 12 +++++- config/observability-rules.yaml | 8 ++-- docs/maintenance-windows.md | 10 ++++- internal/observabilityrules/rules_test.go | 2 +- internal/pressureobserve/compliance.go | 42 ++++++++++++++++++++- internal/pressureobserve/compliance_test.go | 10 +++++ internal/slabheal/slabheal.go | 33 ++++++++++++++++ internal/slabheal/slabheal_test.go | 30 +++++++++++++++ 8 files changed, 138 insertions(+), 9 deletions(-) diff --git a/cmd/gha-fleet/main.go b/cmd/gha-fleet/main.go index bdbacb0c..b3c37124 100644 --- a/cmd/gha-fleet/main.go +++ b/cmd/gha-fleet/main.go @@ -1816,12 +1816,20 @@ func runSlabHeal(args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "gha-fleet: slab-heal: %v\n", err) return 1 } - sunreclaim, err := slabheal.ParseSUnreclaim(meminfo) + counter, err := slabheal.ParseSUnreclaim(meminfo) meminfo.Close() if err != nil { fmt.Fprintf(stderr, "gha-fleet: slab-heal: %v\n", err) return 1 } + attributed, attributedOK := uint64(0), false + if stat, statErr := os.Open("/sys/fs/cgroup/memory.stat"); statErr == nil { + if value, parseErr := slabheal.ParseAttributedSlabUnreclaimable(stat); parseErr == nil { + attributed, attributedOK = value, true + } + stat.Close() + } + sunreclaim, slabSource := slabheal.PreferAttributed(attributed, attributedOK, counter) members, err := client.GetClusterMembers() if err != nil { fmt.Fprintf(stderr, "gha-fleet: slab-heal: read cluster members: %v\n", err) @@ -1870,7 +1878,7 @@ func runSlabHeal(args []string, stdout, stderr io.Writer) int { "reason": decision.Reason, "sunreclaim_bytes": sunreclaim, }) } - reason := fmt.Sprintf("%sSUnreclaim %.1f GiB over the %.1f GiB budget", slabheal.HealReasonPrefix, float64(sunreclaim)/(1<<30), float64(*thresholdBytes)/(1<<30)) + reason := fmt.Sprintf("%sSUnreclaim %.1f GiB (%s) over the %.1f GiB budget", slabheal.HealReasonPrefix, float64(sunreclaim)/(1<<30), slabSource, float64(*thresholdBytes)/(1<<30)) result, err := memberdrain.Drain(ctx, deps, memberdrain.Options{ MemberName: cfg.Incus.Cluster.MemberName, Reason: reason, TimerUnit: *timerUnit, Timeout: *timeout, Poll: *poll, Apply: true, diff --git a/config/observability-rules.yaml b/config/observability-rules.yaml index 667dc971..697fa42f 100644 --- a/config/observability-rules.yaml +++ b/config/observability-rules.yaml @@ -364,7 +364,7 @@ rules: severity: ticket query_language: promql stream_name: system_memory_usage - expression: max(system_memory_usage{service_namespace="nddev-github-actions",state="slab_unreclaimable"}) + expression: max(gha_fleet_host_slab_unreclaimable_attributed_bytes) operator: ">" threshold: 2147483648 evaluation_seconds: 300 @@ -380,9 +380,9 @@ rules: enabled: true owner: fleet-operations runbook: https://github.com/NDDev-OpenNetwork/github-actions/blob/main/docs/runbooks/fleet-alerts.md - summary: Unreclaimable kernel slab exceeds two GiB on a fleet host. - action: Preserve slab, audit and AppArmor evidence; close member admission and roll to the fixed kernel after jobs drain. - recovery: Every fleet host remains below two GiB unreclaimable slab after workload churn. + summary: Unreclaimable kernel slab exceeds two GiB on a fleet host, by memcg-attributed measurement. + action: This watches the root cgroup's attributed slab, not the drifting global counter (github-actions#264 -- the counter overstated truth by ~1 GiB under container churn). A real breach means live kernel objects; preserve slab, audit and AppArmor evidence, then let gha-slab-heal drain and reboot the member. + recovery: Every fleet host remains below two GiB attributed unreclaimable slab after workload churn. - id: kernel_workqueue_hog severity: ticket query_language: promql diff --git a/docs/maintenance-windows.md b/docs/maintenance-windows.md index d03b11b6..75dc992e 100644 --- a/docs/maintenance-windows.md +++ b/docs/maintenance-windows.md @@ -61,7 +61,15 @@ socket every ~11 seconds. Two consequences, both observed: ## The slab healer `gha-slab-heal.timer` (hourly, per member) runs `gha-fleet slab-heal --apply`: -heal only above the alert's own SUnreclaim budget, only with an open gate +heal only above the alert's own SUnreclaim budget — measured as the root +cgroup's **memcg-attributed** `slab_unreclaimable` when cgroup v2 exposes +it, because the global `/proc/meminfo` counter drifts under container churn +(a member showed 1041 MiB there against 44 MiB of attributed truth on +2026-09-01, the gap unattributable to any live cache and immune to +drop_caches and slab shrink; github-actions#264). Both views ship as host +gauges (`…slab_unreclaimable_counter_bytes` / `…_attributed_bytes`) and the +`kernel_slab_unreclaimable` alert watches the attributed one. Then: only +with an open gate (a closed gate belongs to its operator), only when every other member is open, only on a jobless member, only outside a twelve-hour cooldown. The heal drains through the marker, records the cooldown, and reboots only after the diff --git a/internal/observabilityrules/rules_test.go b/internal/observabilityrules/rules_test.go index 872bdee5..ed5a123d 100644 --- a/internal/observabilityrules/rules_test.go +++ b/internal/observabilityrules/rules_test.go @@ -30,7 +30,7 @@ func TestRepositoryRulesUseCurrentMetricSemantics(t *testing.T) { "compute_pressure_observer_missing": `count(up{service_name="pressure-state"} == 1)`, "compute_pressure_state_stale": "gha_fleet_pressure_observer_up", "compute_root_disk_low": "system_filesystem_usage", - "kernel_slab_unreclaimable": `state="slab_unreclaimable"`, + "kernel_slab_unreclaimable": "gha_fleet_host_slab_unreclaimable_attributed_bytes", "audit_suppression_burst": `signal_class="audit_suppressed"`, "kernel_workqueue_hog": `signal_class="kernel_workqueue_hog"`, "host_compliance_observer_missing": "gha_fleet_host_compliance_observer_up", diff --git a/internal/pressureobserve/compliance.go b/internal/pressureobserve/compliance.go index f998e9cb..defa4324 100644 --- a/internal/pressureobserve/compliance.go +++ b/internal/pressureobserve/compliance.go @@ -24,13 +24,34 @@ type Compliance struct { StandardUpdatesAvailable int ESMSecurityUpdatesAvailable int PackageInventoryAgeSeconds float64 + // Two views of unreclaimable slab, exported side by side because the + // global /proc/meminfo counter drifts: on 2026-09-01 a member reported + // 1041 MiB there while the memcg-attributed truth was 44 MiB, the gap + // unattributable to any live cache and immune to drop_caches and slab + // shrink -- a kernel accounting leak under container churn, not memory. + // -1 means the reading was unavailable. + SlabUnreclaimableCounterBytes float64 + SlabUnreclaimableAttributedBytes float64 } func CollectCompliance(root string, now time.Time) Compliance { if root == "" { root = "/" } - result := Compliance{PackageInventoryAgeSeconds: -1, SRSOStatus: "unknown"} + result := Compliance{ + PackageInventoryAgeSeconds: -1, SRSOStatus: "unknown", + SlabUnreclaimableCounterBytes: -1, SlabUnreclaimableAttributedBytes: -1, + } + if meminfo, err := os.ReadFile(filepath.Join(root, "proc", "meminfo")); err == nil { + if value, ok := memoryField(string(meminfo), "SUnreclaim:", 1024); ok { + result.SlabUnreclaimableCounterBytes = value + } + } + if stat, err := os.ReadFile(filepath.Join(root, "sys", "fs", "cgroup", "memory.stat")); err == nil { + if value, ok := memoryField(string(stat), "slab_unreclaimable", 1); ok { + result.SlabUnreclaimableAttributedBytes = value + } + } result.RebootRequired = regularFileExists(filepath.Join(root, "var", "run", "reboot-required")) kernel, kernelErr := os.ReadFile(filepath.Join(root, "proc", "sys", "kernel", "osrelease")) @@ -85,11 +106,30 @@ func RenderCompliance(state Compliance) string { gauge("gha_fleet_host_standard_updates_available", "Standard Ubuntu package updates currently available.", float64(state.StandardUpdatesAvailable)) gauge("gha_fleet_host_esm_security_updates_available", "Additional security updates available only through Ubuntu ESM Apps.", float64(state.ESMSecurityUpdatesAvailable)) gauge("gha_fleet_host_package_inventory_age_seconds", "Age of the update-notifier package inventory, or -1 when unavailable.", state.PackageInventoryAgeSeconds) + gauge("gha_fleet_host_slab_unreclaimable_counter_bytes", "Unreclaimable slab as the drifting global /proc/meminfo counter reports it, or -1 when unavailable.", state.SlabUnreclaimableCounterBytes) + gauge("gha_fleet_host_slab_unreclaimable_attributed_bytes", "Unreclaimable slab as the root cgroup memory.stat attributes it -- the truthful view, or -1 when unavailable.", state.SlabUnreclaimableAttributedBytes) fmt.Fprintf(&output, "# HELP gha_fleet_host_kernel_info Running host kernel identity.\n# TYPE gha_fleet_host_kernel_info gauge\ngha_fleet_host_kernel_info{release=%q} 1\n", escapeLabel(state.KernelRelease)) fmt.Fprintf(&output, "# HELP gha_fleet_host_srso_status Speculative return stack overflow status reported by the running kernel.\n# TYPE gha_fleet_host_srso_status gauge\ngha_fleet_host_srso_status{status=%q} 1\n", escapeLabel(state.SRSOStatus)) return output.String() } +// memoryField finds " " in meminfo/memory.stat content and +// scales it to bytes (meminfo counts KiB, memory.stat counts bytes). +func memoryField(content, key string, scale float64) (float64, bool) { + for _, line := range strings.Split(content, "\n") { + fields := strings.Fields(line) + if len(fields) < 2 || fields[0] != key { + continue + } + value, err := strconv.ParseFloat(fields[1], 64) + if err != nil { + return 0, false + } + return value * scale, true + } + return 0, false +} + func classifySRSO(value string) string { normalized := strings.ToLower(strings.TrimSpace(value)) switch { diff --git a/internal/pressureobserve/compliance_test.go b/internal/pressureobserve/compliance_test.go index 487753fa..d4038827 100644 --- a/internal/pressureobserve/compliance_test.go +++ b/internal/pressureobserve/compliance_test.go @@ -24,6 +24,8 @@ func TestCollectAndRenderCompliance(t *testing.T) { write("sys/devices/system/cpu/vulnerabilities/spec_rstack_overflow", "Vulnerable: Safe RET, no microcode\n") write("var/lib/update-notifier/updates-available", "0 updates can be applied immediately.\n3 additional security updates can be applied with ESM Apps.\n") write("var/run/reboot-required", "*** System restart required ***\n") + write("proc/meminfo", "MemTotal: 16000000 kB\nSUnreclaim: 1066110 kB\nSReclaimable: 200000 kB\n") + write("sys/fs/cgroup/memory.stat", "anon 1\nslab_reclaimable 766643200\nslab_unreclaimable 46323712\nslab 812966912\n") now := time.Now().UTC() state := CollectCompliance(root, now) if !state.Complete || !state.RebootRequired || state.KernelRelease != "6.8.0-138-generic" || state.SRSOStatus != "vulnerable" || state.StandardUpdatesAvailable != 0 || state.ESMSecurityUpdatesAvailable != 3 { @@ -37,6 +39,10 @@ func TestCollectAndRenderCompliance(t *testing.T) { "gha_fleet_host_esm_security_updates_available 3\n", `gha_fleet_host_kernel_info{release="6.8.0-138-generic"} 1`, `gha_fleet_host_srso_status{status="vulnerable"} 1`, + // The drifted counter and the attributed truth ship side by side: + // the 2026-09-01 member showed ~1 GiB against 44 MiB. + "gha_fleet_host_slab_unreclaimable_counter_bytes 1091696640\n", + "gha_fleet_host_slab_unreclaimable_attributed_bytes 46323712\n", } { if !strings.Contains(metrics, wanted) { t.Fatalf("metrics missing %q\n%s", wanted, metrics) @@ -46,6 +52,10 @@ func TestCollectAndRenderCompliance(t *testing.T) { func TestComplianceFailsObservableWhenInputsAreMissing(t *testing.T) { state := CollectCompliance(t.TempDir(), time.Now().UTC()) + if state.SlabUnreclaimableCounterBytes != -1 || state.SlabUnreclaimableAttributedBytes != -1 { + t.Fatalf("absent slab inputs must read -1, got %v / %v", + state.SlabUnreclaimableCounterBytes, state.SlabUnreclaimableAttributedBytes) + } if state.Complete || state.PackageInventoryAgeSeconds != -1 || state.SRSOStatus != "unknown" { t.Fatalf("state=%#v", state) } diff --git a/internal/slabheal/slabheal.go b/internal/slabheal/slabheal.go index a60faafd..5b3b4e71 100644 --- a/internal/slabheal/slabheal.go +++ b/internal/slabheal/slabheal.go @@ -63,6 +63,39 @@ func Decide(facts Facts) Decision { return Decision{Heal: true, Reason: fmt.Sprintf("SUnreclaim %d exceeds the %d budget on a quiet open member", facts.SUnreclaimBytes, facts.ThresholdBytes)} } +// PreferAttributed picks the measurement the guard should act on. The +// global meminfo counter drifts under container churn (measured 1041 MiB +// against a 44 MiB memcg-attributed truth on 2026-09-01, the gap +// unattributable to any live cache), so when the root cgroup exposes +// slab_unreclaimable that attributed value wins; the counter remains the +// fallback for hosts without cgroup v2 memory accounting. +func PreferAttributed(attributed uint64, attributedOK bool, counter uint64) (uint64, string) { + if attributedOK { + return attributed, "memcg-attributed" + } + return counter, "meminfo-counter" +} + +// ParseAttributedSlabUnreclaimable reads slab_unreclaimable (bytes) from +// cgroup v2 memory.stat content. +func ParseAttributedSlabUnreclaimable(reader io.Reader) (uint64, error) { + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + fields := strings.Fields(scanner.Text()) + if len(fields) == 2 && fields[0] == "slab_unreclaimable" { + value, err := strconv.ParseUint(fields[1], 10, 64) + if err != nil { + return 0, fmt.Errorf("malformed slab_unreclaimable value %q", fields[1]) + } + return value, nil + } + } + if err := scanner.Err(); err != nil { + return 0, err + } + return 0, fmt.Errorf("memory.stat has no slab_unreclaimable line") +} + // ParseSUnreclaim reads SUnreclaim from /proc/meminfo content. func ParseSUnreclaim(reader io.Reader) (uint64, error) { scanner := bufio.NewScanner(reader) diff --git a/internal/slabheal/slabheal_test.go b/internal/slabheal/slabheal_test.go index db3d694a..9c1bf836 100644 --- a/internal/slabheal/slabheal_test.go +++ b/internal/slabheal/slabheal_test.go @@ -52,3 +52,33 @@ func TestParseSUnreclaim(t *testing.T) { t.Fatal("missing SUnreclaim line was accepted") } } + +func TestParseAttributedSlabUnreclaimable(t *testing.T) { + t.Parallel() + value, err := ParseAttributedSlabUnreclaimable(strings.NewReader( + "anon 135168\nslab_reclaimable 766643200\nslab_unreclaimable 46323712\nslab 812966912\n", + )) + if err != nil || value != 46323712 { + t.Fatalf("value=%d err=%v", value, err) + } + if _, err := ParseAttributedSlabUnreclaimable(strings.NewReader("anon 1\n")); err == nil { + t.Fatal("missing slab_unreclaimable line must error") + } + if _, err := ParseAttributedSlabUnreclaimable(strings.NewReader("slab_unreclaimable x\n")); err == nil { + t.Fatal("malformed value must error") + } +} + +// The guard must act on the memcg-attributed truth when the kernel exposes +// it: the global counter overstated a member by ~1 GiB on 2026-09-01. +func TestPreferAttributedPicksTruthOverTheDriftingCounter(t *testing.T) { + t.Parallel() + value, source := PreferAttributed(46323712, true, 1091696640) + if value != 46323712 || source != "memcg-attributed" { + t.Fatalf("value=%d source=%s", value, source) + } + value, source = PreferAttributed(0, false, 1091696640) + if value != 1091696640 || source != "meminfo-counter" { + t.Fatalf("fallback value=%d source=%s", value, source) + } +}