fix(admission): exclude reclaimable page cache from the cgroup pressure ratio (#12017)

Real production incident (2026-08-29): the resource-pressure guard ratioed raw cgroup v2 memory.current (which counts reclaimable page cache) against memory.max, so a busy host with ~3GiB of page cache latched a global 503 across every model for 26 minutes even though PSI/OOM/memory.events all showed zero real pressure — the kernel would have reclaimed those pages instantly. Fix: ratio the working set (current - file) for the trip/recovery check, falling back to the raw ratio when memory.stat is missing/stale/zero (never clamping to a false zero-pressure reading).

12 new tests including direct incident reproduction (raw 95%/workingset 32% stays normal) + bug-injection round trips. Full resource-pressure + admission suites green (48/48, re-verified in this batch together with the other 3 PRs: 57/57).
This commit is contained in:
Bob.Hou
2026-08-29 18:52:02 -04:00
committed by GitHub
parent 5698769aba
commit d3420d29f1
9 changed files with 200 additions and 16 deletions

View File

@@ -29,6 +29,7 @@ export type ResourceSignals = {
currentBytes: ResourceMetricBytes;
maxBytes: ResourceMetricBytes;
highBytes: ResourceMetricBytes;
fileBytes: ResourceMetricBytes;
events: {
low: ResourceMetricBytes;
high: ResourceMetricBytes;
@@ -176,9 +177,17 @@ export function classifyAdaptiveResourcePressure(
best,
ratioLevel(signals.v8.heapUsedBytes, signals.v8.heapLimitBytes, thresholds, "v8_heap_ratio")
);
// memory.current includes reclaimable page cache; the kernel drops those
// pages under allocation pressure (memory.events high/max stay 0). Ratio the
// working set (current minus file cache) so cache-heavy-but-healthy hosts do
// not trip the guard. Without memory.stat, fall back to the raw ratio.
// memory.high keeps the raw current: the kernel throttles on TOTAL charge
// (file cache included) when crossing high, so a workingset ratio there
// would miss kernel-side reclaim stalls.
const cgroupWorkingSetBytes = workingSetBytes(signals.cgroup);
best = maxLevel(
best,
ratioLevel(signals.cgroup.currentBytes, signals.cgroup.maxBytes, thresholds, "cgroup_ratio")
ratioLevel(cgroupWorkingSetBytes, signals.cgroup.maxBytes, thresholds, "cgroup_ratio")
);
best = maxLevel(
best,
@@ -188,10 +197,22 @@ export function classifyAdaptiveResourcePressure(
return maxLevel(best, psiLevel(signals.psi?.fullAvg10 ?? null, thresholds, "psi_full"));
}
function workingSetBytes(cgroup: ResourceSignals["cgroup"]): number | null {
if (cgroup.currentBytes == null) return null;
if (cgroup.fileBytes == null || cgroup.fileBytes <= 0) return cgroup.currentBytes;
// memory.current and memory.stat are separate, non-atomic reads; under churn
// file can momentarily exceed a fresher current. Treat that as a bad sample
// and fall back to the raw ratio rather than clamping to 0, which would
// read as zero pressure and could force a premature recovery.
if (cgroup.fileBytes > cgroup.currentBytes) return cgroup.currentBytes;
return cgroup.currentBytes - cgroup.fileBytes;
}
function isRecovered(signals: ResourceSignals, thresholds: ResourcePressureThresholds): boolean {
const cgroupWorkingSetBytes = workingSetBytes(signals.cgroup);
const ratios: Array<readonly [number | null, number | null]> = [
[signals.v8.heapUsedBytes, signals.v8.heapLimitBytes],
[signals.cgroup.currentBytes, signals.cgroup.maxBytes],
[cgroupWorkingSetBytes, signals.cgroup.maxBytes],
[signals.cgroup.currentBytes, signals.cgroup.highBytes],
];
if (

View File

@@ -180,6 +180,22 @@ function parsePsiNumber(line: string, name: string): number | null {
return Number.isFinite(parsed) && parsed >= 0 ? parsed : null;
}
function parseMemoryStatFileBytes(text: string | null): number | null {
if (!text) return null;
for (const line of text.split("\n")) {
const [key, rawValue] = line.trim().split(/\s+/, 2);
if (key !== "file" || rawValue == null) continue;
// sanitizeMemoryBytes rejects every falsy magnitude INCLUDING zero, but a
// zero file cache is a valid reading (workingSetBytes falls back to the
// raw ratio for it). The zero short-circuit below is coupled to that
// contract on purpose; if sanitizeMemoryBytes ever accepts zero, this
// branch becomes dead but harmless.
if (rawValue === "0") return 0;
return sanitizeMemoryBytes(rawValue);
}
return null;
}
function parsePsi(text: string | null): ResourceSignals["psi"] {
if (!text) return null;
const result: NonNullable<ResourceSignals["psi"]> = {
@@ -226,14 +242,25 @@ export async function sampleResourceSignals(
}
const cgroupDirectory = await resolveCgroupDirectory(readText);
const cgroupContents = cgroupDirectory
const cgroupReads = cgroupDirectory
? await Promise.all([
readText(path.join(cgroupDirectory, "memory.current")),
readText(path.join(cgroupDirectory, "memory.max")),
readText(path.join(cgroupDirectory, "memory.high")),
readText(path.join(cgroupDirectory, "memory.events")),
readText(path.join(cgroupDirectory, "memory.stat")),
])
: [null, null, null, null];
: null;
// Named bindings instead of positional indices: the read order above is
// easy to shuffle on edit, and a silent index shift would corrupt the
// current/max/high/stat mapping.
const cgroupFiles = {
current: cgroupReads?.[0] ?? null,
max: cgroupReads?.[1] ?? null,
high: cgroupReads?.[2] ?? null,
events: cgroupReads?.[3] ?? null,
stat: cgroupReads?.[4] ?? null,
};
const psi = await readText("/proc/pressure/memory").catch(() => null);
return {
@@ -247,10 +274,11 @@ export async function sampleResourceSignals(
constrainedBytes: safeNumber(deps.constrainedMemory ?? (() => process.constrainedMemory?.())),
},
cgroup: {
currentBytes: sanitizeMemoryBytes(cgroupContents[0]),
maxBytes: sanitizeMemoryBytes(cgroupContents[1]),
highBytes: sanitizeMemoryBytes(cgroupContents[2]),
events: parseMemoryEvents(cgroupContents[3]),
currentBytes: sanitizeMemoryBytes(cgroupFiles.current),
maxBytes: sanitizeMemoryBytes(cgroupFiles.max),
highBytes: sanitizeMemoryBytes(cgroupFiles.high),
fileBytes: parseMemoryStatFileBytes(cgroupFiles.stat),
events: parseMemoryEvents(cgroupFiles.events),
},
psi: parsePsi(psi),
};

View File

@@ -161,7 +161,7 @@ function reloadNormalResourcePressure() {
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
}),
});

View File

@@ -37,7 +37,7 @@ function reloadNormalResourcePressure() {
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
}),
});

View File

@@ -41,7 +41,7 @@ async function resetStorage() {
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
}),
});
@@ -157,7 +157,7 @@ test("direct handleChatCore default still applies resource pressure guard", asyn
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
}),
});
@@ -198,7 +198,7 @@ test("handleChatCore skipResourcePressureGuard bypasses the inside-core fuse", a
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
}),
});

View File

@@ -23,7 +23,7 @@ function baseSignals(overrides: Partial<ResourceSignals> = {}): ResourceSignals
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
...overrides,
};
@@ -133,6 +133,7 @@ describe("resource pressure policy", () => {
currentBytes: null,
maxBytes: null,
highBytes: null,
fileBytes: null,
events: { low: 0, high: 0, max: 0, oom, oom_kill },
},
});
@@ -160,6 +161,7 @@ describe("resource pressure policy", () => {
currentBytes: null,
maxBytes: null,
highBytes: null,
fileBytes: null,
events: { low: 0, high: 0, max: 0, oom, oom_kill },
},
});
@@ -174,6 +176,110 @@ describe("resource pressure policy", () => {
assert.equal(tracker.observe(events(9, 3)).severity, "normal", "replacement re-baselines");
});
it("ignores reclaimable page cache in the cgroup workingset ratio", () => {
// Incident 2026-08-29: memory.current included 3.01 GiB of reclaimable
// page cache on top of 1.62 GiB anon, tripping cgroup_ratio critical at
// 95% while the real working set was 33% and memory.events stayed zero.
const tracker = createResourcePressureTracker(fastThresholds);
const cgroup = (
currentBytes: number,
fileBytes: number | null
): ResourceSignals["cgroup"] => ({
currentBytes,
maxBytes: 5 * 1024 ** 3,
highBytes: null,
fileBytes,
events: { low: 0, high: 0, max: 0, oom: 0, oom_kill: 0 },
});
const signals = (c: ResourceSignals["cgroup"]): ResourceSignals => ({
...baseSignals(),
cgroup: c,
});
// Raw current at 95% with 3 GiB reclaimable cache: workingset is 1.62/5 = 32%.
assert.equal(tracker.observe(signals(cgroup(5_033_164_800, 3_232_225_280))).severity, "normal");
// Same current with zero cache is genuine pressure (sustained 2 samples).
tracker.observe(signals(cgroup(5_033_164_800, 0)));
assert.equal(tracker.observe(signals(cgroup(5_033_164_800, 0))).severity, "critical");
// Missing memory.stat (fileBytes null) keeps the legacy raw-ratio behavior.
tracker.observe(signals(cgroup(5_033_164_800, null)));
assert.equal(tracker.observe(signals(cgroup(5_033_164_800, null))).severity, "critical");
});
it("recovers the cgroup_ratio latch once page cache drains", () => {
const tracker = createResourcePressureTracker(fastThresholds);
const cgroup = (currentBytes: number, fileBytes: number | null): ResourceSignals => ({
...baseSignals(),
cgroup: {
currentBytes,
maxBytes: 5 * 1024 ** 3,
highBytes: null,
fileBytes,
events: { low: 0, high: 0, max: 0, oom: 0, oom_kill: 0 },
},
});
let state = tracker.observe(cgroup(5_033_164_800, 0)); // genuine critical
state = tracker.observe(cgroup(5_033_164_800, 0)); // sustained 2 samples
assert.equal(state.severity, "critical");
// Cache grows while anon stays low: current stays high but workingset drops.
state = tracker.observe(cgroup(4_662_461_440, 3_232_225_280));
assert.equal(state.severity, "critical", "recovery needs sustained samples");
state = tracker.observe(cgroup(4_662_461_440, 3_232_225_280));
assert.equal(state.severity, "normal", "workingset below recovery ratio releases the latch");
});
it("handles workingset boundary conditions", () => {
const tracker = createResourcePressureTracker(fastThresholds);
const mk = (cur: number, file: number | null): ResourceSignals => ({
...baseSignals(),
cgroup: {
currentBytes: cur,
maxBytes: 5 * 1024 ** 3,
highBytes: null,
fileBytes: file,
events: { low: 0, high: 0, max: 0, oom: 0, oom_kill: 0 },
},
});
// measurement skew: file > current falls back to the raw ratio, never 0.
// Raw current 95% with a bogus file reading must still read as critical.
tracker.observe(mk(5_033_164_800, 5_999_000_000));
let state = tracker.observe(mk(5_033_164_800, 5_999_000_000));
assert.equal(state.severity, "critical");
// file = 0 is a valid stat read (no page cache): raw ratio path.
tracker.observe(mk(5_033_164_800, 0));
state = tracker.observe(mk(5_033_164_800, 0));
assert.equal(state.severity, "critical");
// file exactly equal to current: workingset is 0 (all cache), ratio 0.
tracker.observe(mk(5_033_164_800, 5_033_164_800));
state = tracker.observe(mk(5_033_164_800, 5_033_164_800));
assert.equal(state.severity, "normal");
});
it("keeps the cgroup_high reason on the raw total charge", () => {
const tracker = createResourcePressureTracker(fastThresholds);
const mk = (cur: number, file: number, high: number): ResourceSignals => ({
...baseSignals(),
cgroup: {
currentBytes: cur,
maxBytes: 5 * 1024 ** 3,
highBytes: high,
fileBytes: file,
events: { low: 0, high: 0, max: 0, oom: 0, oom_kill: 0 },
},
});
// Total charge 3.5 GiB over a 3 GiB high with 3 GiB of it file cache:
// kernel throttles on the total, so the guard must fire on cgroup_high
// even though the workingset (0.5 GiB) is tiny.
tracker.observe(mk(3_758_096_384, 3_221_225_472, 3 * 1024 ** 3));
const state = tracker.observe(mk(3_758_096_384, 3_221_225_472, 3 * 1024 ** 3));
assert.equal(state.severity, "critical");
assert.equal(state.reason, "cgroup_high");
});
it("keeps snapshot state fields and bounded-cardinality values", () => {
const tracker = createResourcePressureTracker(fastThresholds);
const state: ResourcePressureState = tracker.observe(baseSignals());

View File

@@ -19,7 +19,7 @@ function signals(observedAtMs: number, heapUsedMb = 100): ResourceSignals {
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
};
}

View File

@@ -92,6 +92,29 @@ describe("resource pressure cgroup parsers", () => {
});
describe("sampleResourceSignals", () => {
it("keeps an explicit zero file-cache reading distinct from unavailable", async () => {
const fs = mapFs([
["/proc/self/cgroup", "0::/slice/service\n"],
["/proc/self/mountinfo", "43 34 0:35 / /sys/fs/cgroup rw - cgroup2 cgroup2 rw\n"],
["/sys/fs/cgroup/slice/service/memory.current", `${800 * MiB}\n`],
["/sys/fs/cgroup/slice/service/memory.max", `${GiB}\n`],
["/sys/fs/cgroup/slice/service/memory.high", "966367641\n"],
["/sys/fs/cgroup/slice/service/memory.events", "low 0\nhigh 0\nmax 0\noom 0\noom_kill 0\n"],
["/sys/fs/cgroup/slice/service/memory.stat", "anon 268435456\nfile 0\n"],
[
"/proc/pressure/memory",
"some avg10=1.50 avg60=2.00 avg300=3.25 total=9\nfull avg10=0.25 avg60=0.50 avg300=0.75 total=1\n",
],
]);
const signals = await sampleResourceSignals({
nowMs: () => 42,
memoryUsage: () => memoryUsage(250 * MiB),
heapStatistics: () => ({ heap_size_limit: GiB, used_heap_size: 250 * MiB }),
fs,
});
assert.equal(signals.cgroup.fileBytes, 0);
});
it("captures process, V8, cgroup, event, and PSI snapshot fields", async () => {
const fs = mapFs([
["/proc/self/cgroup", "0::/slice/service\n"],
@@ -100,6 +123,10 @@ describe("sampleResourceSignals", () => {
["/sys/fs/cgroup/slice/service/memory.max", `${GiB}\n`],
["/sys/fs/cgroup/slice/service/memory.high", "966367641\n"],
["/sys/fs/cgroup/slice/service/memory.events", "low 1\nhigh 2\nmax 3\noom 4\noom_kill 5\n"],
[
"/sys/fs/cgroup/slice/service/memory.stat",
`anon 268435456\nfile ${300 * MiB}\nkernel_stack 1048576\n`,
],
[
"/proc/pressure/memory",
"some avg10=1.50 avg60=2.00 avg300=3.25 total=9\nfull avg10=0.25 avg60=0.50 avg300=0.75 total=1\n",
@@ -128,6 +155,7 @@ describe("sampleResourceSignals", () => {
currentBytes: 800 * MiB,
maxBytes: GiB,
highBytes: 966367641,
fileBytes: 300 * MiB,
events: { low: 1, high: 2, max: 3, oom: 4, oom_kill: 5 },
});
assert.equal(signals.psi?.someAvg10, 1.5);
@@ -155,6 +183,7 @@ describe("sampleResourceSignals", () => {
currentBytes: null,
maxBytes: null,
highBytes: null,
fileBytes: null,
events: null,
});
assert.equal(signals.psi, null);

View File

@@ -20,7 +20,7 @@ function signals(observedAtMs: number, heapUsedMb: number): ResourceSignals {
availableBytes: null,
constrainedBytes: null,
},
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, events: null },
cgroup: { currentBytes: null, maxBytes: null, highBytes: null, fileBytes: null, events: null },
psi: null,
};
}