Public: anyone can read this; the crew can change it.
A memory cut names its real source: FORGE_CELL_MEMORY_MAX_MIB only when set, otherwise the pod's memory.max share #3158
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3145-a-memory-cut-names-its-real-source-forge → main 1 commit fork 3 threads · 3 unresolved
3 files
changed +56
−10
src/apps/contain.ts +24 −8
| @@ -40,6 +40,7 @@ import { relay, startEgress, type Egress } from "./egress.ts"; | ||
| 40 | 40 | import { cgroupHost, enterArgv, openCellCgroup } from "./cgroup.ts"; |
| 41 | 41 | |
| 42 | 42 | /** `ceiling`: a declaration was cut to the machine's maximum; `declared`: forge.json's own; `default`: neither. */ |
| 43 | export type CeilingFrom = "env" | "pod" | "default"; | |
| 43 | 44 | export type MemorySource = "ceiling" | "declared" | "default"; |
| 44 | 45 | |
| 45 | 46 | export interface Limits { |
| @@ -47,6 +48,8 @@ export interface Limits { | ||
| 47 | 48 | readonly memoryMiB: number; |
| 48 | 49 | /** Where `memoryMiB` came from, for the line that says it was exceeded. */ |
| 49 | 50 | readonly memorySource?: MemorySource | undefined; |
| 51 | /** Where the machine's memory ceiling came from, named when `memorySource` is `ceiling`. */ | |
| 52 | readonly ceilingFrom?: CeilingFrom | undefined; | |
| 50 | 53 | /** Cores, averaged over `CPU_WINDOW_MS`. */ |
| 51 | 54 | readonly cpus: number; |
| 52 | 55 | /** Bytes on disk under the cell's writable directories, MiB. */ |
| @@ -93,14 +96,14 @@ export function memoryCeiling( | ||
| 93 | 96 | export function memoryCeilingSource( |
| 94 | 97 | env: string | undefined, |
| 95 | 98 | podFile = POD_MEMORY_FILE, |
| 96 | ): "env" | "pod" | "default" { | |
| 99 | ): CeilingFrom { | |
| 97 | 100 | return ceiling(env, podFile).from; |
| 98 | 101 | } |
| 99 | 102 | |
| 100 | 103 | function ceiling( |
| 101 | 104 | env: string | undefined, |
| 102 | 105 | podFile: string, |
| 103 | ): { mib: number; from: "env" | "pod" | "default" } { | |
| 106 | ): { mib: number; from: CeilingFrom } { | |
| 104 | 107 | const set = positive(env); |
| 105 | 108 | if (set !== undefined) return { mib: set, from: "env" }; |
| 106 | 109 | try { |
| @@ -114,18 +117,30 @@ function ceiling( | ||
| 114 | 117 | } |
| 115 | 118 | |
| 116 | 119 | /** The line saying a declared memory limit is cut to the machine's ceiling; none when it is not. */ |
| 117 | export function limitWarnings(declared: DeclaredLimits | undefined): string[] { | |
| 120 | export function limitWarnings( | |
| 121 | declared: DeclaredLimits | undefined, | |
| 122 | podFile = POD_MEMORY_FILE, | |
| 123 | ): string[] { | |
| 118 | 124 | const want = declared?.memoryMiB; |
| 119 | const have = maxLimits().memoryMiB; | |
| 125 | const have = maxLimits(podFile).memoryMiB; | |
| 120 | 126 | return want !== undefined && want > have |
| 121 | ? [`warning: forge.json declares memory ${want} MiB; this machine grants at most ${have} (FORGE_CELL_MEMORY_MAX_MIB, else a share of the pod's memory)`] | |
| 127 | ? [`warning: forge.json declares memory ${want} MiB; this machine grants at most ${have} (${ceilingText(memoryCeilingSource(process.env["FORGE_CELL_MEMORY_MAX_MIB"], podFile))})`] | |
| 122 | 128 | : []; |
| 123 | 129 | } |
| 124 | 130 | |
| 131 | /** The ceiling's real source in words: only the variable that is set is named. */ | |
| 132 | function ceilingText(from: CeilingFrom | undefined): string { | |
| 133 | return from === "env" | |
| 134 | ? "FORGE_CELL_MEMORY_MAX_MIB" | |
| 135 | : from === "pod" | |
| 136 | ? `${POD_MEMORY_SHARE * 100}% of the pod's memory.max` | |
The default text hardcodes 4096. If the default in memoryCeiling changes, this line goes stale. Interpolate the shared constant. |
||
| 137 | : "the 4096 MiB default, the host states no memory limit"; | |
| 138 | } | |
| 139 | ||
| 125 | 140 | /** Where a cell's memory limit came from, for the line that says it was exceeded. */ |
| 126 | export function memorySource(source: MemorySource | undefined): string { | |
| 141 | export function memorySource(source: MemorySource | undefined, from?: CeilingFrom): string { | |
| 127 | 142 | return source === "ceiling" |
| 128 | ? "the machine's ceiling, FORGE_CELL_MEMORY_MAX_MIB; a higher forge.json limit was held to it" | |
| 143 | ? `the machine's ceiling, ${ceilingText(from)}; a higher forge.json limit was held to it` | |
| 129 | 144 | : source === "declared" |
| 130 | 145 | ? "forge.json limits.memory" |
| 131 | 146 | : "the default"; |
| @@ -148,6 +163,7 @@ export function resolveLimits( | ||
| 148 | 163 | return { |
| 149 | 164 | memoryMiB: Math.min(want ?? base.memoryMiB, max.memoryMiB), |
| 150 | 165 | memorySource: want === undefined ? "default" : want > max.memoryMiB ? "ceiling" : "declared", |
| 166 | ceilingFrom: memoryCeilingSource(process.env["FORGE_CELL_MEMORY_MAX_MIB"], podFile), | |
| 151 | 167 | cpus: Math.min(declared?.cpus ?? base.cpus, max.cpus), |
| 152 | 168 | diskMiB: Math.min(declared?.diskMiB ?? base.diskMiB, max.diskMiB), |
| 153 | 169 | }; |
| @@ -889,7 +905,7 @@ function tick(): void { | ||
| 889 | 905 | entry.last = { ticks, kib }; |
| 890 | 906 | if (kib > limits.memoryMiB * 1024) { |
| 891 | 907 | entry.breach( |
| 892 | `used ${mib(kib * 1024)} of memory, over its ${limits.memoryMiB} MiB limit (${memorySource(limits.memorySource)})`, | |
| 908 | `used ${mib(kib * 1024)} of memory, over its ${limits.memoryMiB} MiB limit (${memorySource(limits.memorySource, limits.ceilingFrom)})`, | |
| 893 | 909 | ); |
| 894 | 910 | continue; |
| 895 | 911 | } |
test/check-memory-warning.test.ts +1 −1
| @@ -83,7 +83,7 @@ test("the breach text names where the limit came from", async () => { | ||
| 83 | 83 | ); |
| 84 | 84 | try { |
| 85 | 85 | const why = await new Promise<string>((resolve) => { |
| 86 | const limits: Limits = { memoryMiB: 64, memorySource: "ceiling", cpus: 64, diskMiB: 1 << 20 }; | |
| 86 | const limits: Limits = { memoryMiB: 64, memorySource: "ceiling", ceilingFrom: "env", cpus: 64, diskMiB: 1 << 20 }; | |
| 87 | 87 | const unwatch = watchTree(hog.pid, limits, { bytes: 64 << 20, by: "watch" }, [], undefined, (w) => { |
| 88 | 88 | unwatch(); |
| 89 | 89 | resolve(w); |
test/memory-ceiling.test.ts +31 −1
| @@ -40,7 +40,10 @@ describe("the memory ceiling is never a silent clamp (#3128)", () => { | ||
| 40 | 40 | process.env["FORGE_CELL_MEMORY_MAX_MIB"] = "4096"; |
| 41 | 41 | try { |
| 42 | 42 | const source = (declared?: { memoryMiB: number }) => |
| 43 | memorySource(resolveLimits("step", declared).memorySource); | |
| 43 | { | |
The arrow body is wrapped in a stray block with odd indentation. Use a plain block body on the arrow. |
||
| 44 | const l = resolveLimits("step", declared); | |
| 45 | return memorySource(l.memorySource, l.ceilingFrom); | |
| 46 | }; | |
| 44 | 47 | expect(source({ memoryMiB: 8192 })).toContain("FORGE_CELL_MEMORY_MAX_MIB"); |
| 45 | 48 | expect(source({ memoryMiB: 4096 })).toBe("forge.json limits.memory"); |
| 46 | 49 | expect(source({ memoryMiB: 1024 })).toBe("forge.json limits.memory"); |
| @@ -51,4 +54,31 @@ describe("the memory ceiling is never a silent clamp (#3128)", () => { | ||
| 51 | 54 | else process.env["FORGE_CELL_MEMORY_MAX_MIB"] = saved; |
| 52 | 55 | } |
| 53 | 56 | }); |
| 57 | ||
| 58 | test("each source is named by the warning and the kill message, and only a set variable is called a variable", () => { | |
| 59 | const saved = process.env["FORGE_CELL_MEMORY_MAX_MIB"]; | |
| 60 | const small = pod(String(4096 * 1048576)); | |
| 61 | try { | |
| 62 | process.env["FORGE_CELL_MEMORY_MAX_MIB"] = "1024"; | |
| 63 | expect(limitWarnings({ memoryMiB: 2048 }, small)[0]).toContain("FORGE_CELL_MEMORY_MAX_MIB"); | |
| 64 | expect(memorySource("ceiling", resolveLimits("step", { memoryMiB: 2048 }, small).ceilingFrom)).toContain("FORGE_CELL_MEMORY_MAX_MIB"); | |
| 65 | ||
| 66 | delete process.env["FORGE_CELL_MEMORY_MAX_MIB"]; | |
| 67 | const podLine = limitWarnings({ memoryMiB: 8192 }, small)[0] ?? ""; | |
| 68 | expect(podLine).toContain("75% of the pod's memory.max"); | |
| 69 | expect(podLine).not.toContain("FORGE_CELL_MEMORY_MAX_MIB"); | |
| 70 | const podKill = memorySource("ceiling", resolveLimits("step", { memoryMiB: 8192 }, small).ceilingFrom); | |
| 71 | expect(podKill).toContain("75% of the pod's memory.max"); | |
| 72 | expect(podKill).not.toContain("FORGE_CELL_MEMORY_MAX_MIB"); | |
| 73 | ||
| 74 | const none = pod("max\n"); | |
| 75 | const defLine = limitWarnings({ memoryMiB: 8192 }, none)[0] ?? ""; | |
| 76 | expect(defLine).toContain("4096 MiB default"); | |
| 77 | expect(defLine).not.toContain("FORGE_CELL_MEMORY_MAX_MIB"); | |
| 78 | expect(memorySource("ceiling", "default")).not.toContain("FORGE_CELL_MEMORY_MAX_MIB"); | |
| 79 | } finally { | |
| 80 | if (saved === undefined) delete process.env["FORGE_CELL_MEMORY_MAX_MIB"]; | |
| 81 | else process.env["FORGE_CELL_MEMORY_MAX_MIB"] = saved; | |
| 82 | } | |
| 83 | }); | |
| 54 | 84 | }); |
new above
offline
The doc comment for MemorySource (ceiling/declared/default) now sits above CeilingFrom. Put it back above MemorySource and give CeilingFrom its own one-line comment.