From abe03b1a2987960b9363708b3f04726446bd3941 Mon Sep 17 00:00:00 2001 From: David McKay Date: Wed, 28 Oct 2026 18:12:53 -0700 Subject: [PATCH 1/3] Close a computer's browsers when its memory runs high, and a Bot's idle background tabs A computer capped its browsers by count and closed them after half an hour untouched, and neither looked at memory. A few heavy pages could take a container past its limit, and on Kubernetes the OOM killer then kills everything in the container together, so every Bot's browser went at once. The computer now reads its own working set the way the kubelet does: memory.current less inactive_file on cgroup v2, the v1 files where only those exist, measured against the machine's memory when the container has no limit, and nothing at all outside a container. Before each launch and on each idle-sweep tick, at or above COMPUTER_MEMORY_HIGH_PERCENT (default 85) it closes browsers one at a time, least recently used first by the cap's own order, reading memory again after each. It never closes the browser being launched, the most recently used one, or the browser of a Bot whose wheel a person holds or has been asked to take. Each close logs computer-browser-closed-for-memory with the working set and the limit. The idle sweep also closes a Bot's background tabs that have gone untouched for COMPUTER_TAB_IDLE_MS (default ten minutes). A Bot is always on its newest page, so an older tab is one it cannot return to. The page the Bot is on, a person's own browser, extension and DevTools pages, and any tab of a Bot whose wheel a person holds are never closed. There is no per-browser tab count: memory is now watched directly, and a count would close tabs a site opened moments ago. Both settings are wired through the chart, compose, the all-in-one image and the docs. The chart renders a zero and a whole number from a values file as the computer reads them, which the new-values-key check now asserts on every target that runs the shared computer. --- .env.example | 12 + .github/workflows/ci.yml | 2 +- CHANGELOG.md | 31 ++- agent-computer/src/browser-eviction.ts | 104 +++++++- agent-computer/src/container-memory.ts | 180 ++++++++++++++ agent-computer/src/index.ts | 9 +- agent-computer/src/profiles.ts | 215 +++++++++++++++- agent-computer/src/sessions.ts | 30 ++- agent-computer/tests/browser-eviction.test.ts | 147 ++++++++++- agent-computer/tests/browser-memory.test.ts | 232 ++++++++++++++++++ agent-computer/tests/container-memory.test.ts | 121 +++++++++ .../v1-no-limit/memory/memory.limit_in_bytes | 1 + .../cgroup/v1-no-limit/memory/memory.stat | 36 +++ .../v1-no-limit/memory/memory.usage_in_bytes | 1 + .../cgroup/v2-limit-512m/memory.current | 1 + .../fixtures/cgroup/v2-limit-512m/memory.max | 1 + .../fixtures/cgroup/v2-limit-512m/memory.stat | 61 +++++ .../cgroup/v2-no-limit/memory.current | 1 + .../fixtures/cgroup/v2-no-limit/memory.max | 1 + .../fixtures/cgroup/v2-no-limit/memory.stat | 61 +++++ .../cgroup/v2-page-cache/memory.current | 1 + .../fixtures/cgroup/v2-page-cache/memory.max | 1 + .../fixtures/cgroup/v2-page-cache/memory.stat | 61 +++++ .../cgroup/v2-shm-near-limit/memory.current | 1 + .../cgroup/v2-shm-near-limit/memory.max | 1 + .../cgroup/v2-shm-near-limit/memory.stat | 61 +++++ agent-computer/tests/sessions.test.ts | 18 ++ charts/openbot/templates/_helpers.tpl | 13 + .../templates/computer/statefulset.yaml | 13 + charts/openbot/values.yaml | 9 + docker-compose.yml | 5 + docker/s6/s6-rc.d/computer/run | 2 +- docs/configuration.md | 2 + scripts/check-new-values-keys.ts | 72 ++++++ 34 files changed, 1481 insertions(+), 26 deletions(-) create mode 100644 agent-computer/src/container-memory.ts create mode 100644 agent-computer/tests/browser-memory.test.ts create mode 100644 agent-computer/tests/container-memory.test.ts create mode 100644 agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.limit_in_bytes create mode 100644 agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.stat create mode 100644 agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.usage_in_bytes create mode 100644 agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.current create mode 100644 agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.max create mode 100644 agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.stat create mode 100644 agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.current create mode 100644 agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.max create mode 100644 agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.stat create mode 100644 agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.current create mode 100644 agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.max create mode 100644 agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.stat create mode 100644 agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.current create mode 100644 agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.max create mode 100644 agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.stat diff --git a/.env.example b/.env.example index 0ff9ee6da2..fb4bb07c8d 100644 --- a/.env.example +++ b/.env.example @@ -359,6 +359,18 @@ COMPUTER_TOKEN= # COMPUTER_BROWSER_BACKEND=managed # COMPUTER_BROWSER_MODE=headed +# When a computer closes a browser besides the cap (COMPUTER_MAX_BROWSERS) and the idle timeout +# (COMPUTER_BROWSER_IDLE_MS). At or above this share of its memory limit, read as the kubelet reads it +# (the working set, so the page cache does not count), it closes browsers least recently used first +# until it is back under, so a few heavy pages cost one Bot a relaunch rather than the OOM killer +# taking every browser at once. A container with no memory limit is measured against the machine's +# memory; a computer outside a container closes nothing for memory. 0 switches it off. +# COMPUTER_MEMORY_HIGH_PERCENT=85 +# How long a Bot's background tab may sit untouched before it is closed, in ms: a tab the Bot left +# behind, such as the page a sign-in popup opened from. The page the Bot is on, a person's own +# browser, and any tab of a Bot whose wheel a person holds are never closed. 0 keeps them. +# COMPUTER_TAB_IDLE_MS=600000 + # Web Bot Auth: every request a Bot's browser makes is signed as coming from this deployment # (Ed25519, HTTP Message Signatures), and the public key is published at # /.well-known/http-message-signatures-directory. Off unless set to on, because diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe0c0082ad..228422a2cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,7 +141,7 @@ jobs: bun test tests/sign-in.test.ts tests/session-cookies.test.ts tests/follows-popup.test.ts tests/demonstration-live.test.ts tests/webrtc-proxy.test.ts tests/web-bot-auth-browser.test.ts tests/handoff-page-restore.test.ts tests/enter-text.test.ts tests/person-browser.test.ts tests/person-browser-computer.test.ts - tests/screencast-keys-browser.test.ts + tests/screencast-keys-browser.test.ts tests/browser-memory.test.ts working-directory: agent-computer env: OPENBOT_COMPUTER_BROWSER: '1' diff --git a/CHANGELOG.md b/CHANGELOG.md index e70e370555..4528022303 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased -**Before upgrading.** Fourteen things change for an existing deployment: +**Before upgrading.** Fifteen things change for an existing deployment: - A hosted computer's browser is now headed by default. It uses a little more memory than headless did; set `COMPUTER_BROWSER_MODE=headless` (Helm: `computers.browserMode: headless`) to keep the old behaviour. @@ -61,6 +61,35 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. to keep the old behaviour. - Interfaces a Bot drew before the upgrade (A2UI 0.9 surfaces) no longer draw when a conversation is reopened; the messages around them stay. New ones use A2UI 1.0 (below). +- A computer now closes browsers, least recently used first, when its memory reaches 85% of its + limit, and closes a Bot's background tabs after ten minutes untouched (below). + `COMPUTER_MEMORY_HIGH_PERCENT=0` and `COMPUTER_TAB_IDLE_MS=0` (Helm: `computers.memoryHighPercent` + and `computers.tabIdleMs`) keep the old behaviour. + +### A computer closes a browser before its memory runs out + +A computer runs one browser for each Bot that is browsing, and the only limits were a count and half +an hour untouched. Neither looked at memory, so a few heavy pages could take a container past its +memory limit well under the count, and the kernel then killed processes without regard to which Bot +was busy. On Kubernetes it kills everything in the container at once, so every Bot's browser closed +together and nothing could reach the computer until it came back. A computer now reads its own +memory the way the kubelet does (the working set, which leaves out the file cache the kernel can +drop), before it starts a browser and once a minute. At or above 85% of its limit it closes browsers +one at a time, least recently used first, until it is back under. A Bot whose browser was closed +starts again where it left off on its next step, still signed in, because its profile is on disk. +The browser being started, the one used most recently, and the browser of a Bot whose wheel a person +holds or has been asked to take are never closed for memory. A container with no memory limit is +measured against the machine's memory, and a computer outside a container, such as OpenBot +Desktop's, closes nothing for memory. Each close is logged as `computer-browser-closed-for-memory` +with the working set and the limit in bytes. `COMPUTER_MEMORY_HIGH_PERCENT` (Helm: +`computers.memoryHighPercent`) sets the share, and `0` switches it off. + +The tabs a Bot leaves behind are closed too. A Bot always works in its newest page, so an older tab, +such as the page a sign-in popup was opened from or a link a site opened in a new tab, is one it +cannot go back to, and it held memory for as long as the browser stayed up. A background tab that +nothing has used, navigated or brought to the front for ten minutes is now closed. The page the Bot +is on is never closed, and neither is any tab of a person's own browser or of a Bot whose wheel a +person holds. `COMPUTER_TAB_IDLE_MS` (Helm: `computers.tabIdleMs`) sets the time, and `0` keeps them. ### Generated interfaces use A2UI 1.0 diff --git a/agent-computer/src/browser-eviction.ts b/agent-computer/src/browser-eviction.ts index be87a70883..e0ae9989dd 100644 --- a/agent-computer/src/browser-eviction.ts +++ b/agent-computer/src/browser-eviction.ts @@ -1,5 +1,5 @@ /** - * Which Bots' browsers to close, and when. + * Which Bots' browsers to close, and when, and which of a Bot's tabs. * * Its own file, with no Playwright import, for the reason `authorisation.ts` and `bot-id.ts` are: * `profiles.ts` imports Playwright at module scope, so anything living there needs a browser merely @@ -62,21 +62,66 @@ export function chooseEvictions( const entries = [...running]; if (entries.length <= max) return []; // A cap of zero keeps nothing, the newest included; that is the mistake reading the setting guards. - const newest = - max > 0 - ? entries.reduce((latest, entry) => - entry[1].usedAt >= latest[1].usedAt ? entry : latest, - ) - : undefined; + const newest = max > 0 ? newestOf(entries) : undefined; + return inClosingOrder(entries, newest, goesLast) + .slice(0, entries.length - max) + .map(([botId]) => botId); +} + +/** The most recently used, which is the one a launch has just added. Ties go to the later entry. */ +function newestOf( + entries: [string, Evictable][], +): [string, Evictable] | undefined { + if (entries.length === 0) return undefined; + return entries.reduce((latest, entry) => + entry[1].usedAt >= latest[1].usedAt ? entry : latest, + ); +} + +/** + * The order browsers are closed in when some have to go: every Bot's before a person's native window + * (`keptOpen`), and least recently used first within each. `spare` is left out, which is the newest. + */ +function inClosingOrder( + entries: [string, Evictable][], + spare: [string, Evictable] | undefined, + goesLast: (botId: string) => boolean, +): [string, Evictable][] { return entries - .filter((entry) => entry !== newest) + .filter((entry) => entry !== spare) .sort( ([aId, a], [bId, b]) => Number(goesLast(aId)) - Number(goesLast(bId)) || a.usedAt - b.usedAt, - ) - .slice(0, entries.length - max) - .map(([botId]) => botId); + ); +} + +/** + * Which to close next because the computer's memory runs high (`container-memory.ts`). + * + * One at a time, because the caller reads memory again after each close and stops as soon as it is + * enough: a count says in advance how many have to go, and bytes do not. + * + * The cap's order, so the two never disagree about who goes first: least recently used, with a + * person's native window last. And the cap's protection: the newest is never chosen. Before a + * launch the caller counts the Bot that is asking as the newest, so the browser being launched is + * never what makes room for itself; on the idle sweep the newest is whoever was used last. Either + * way at least one browser is always left, because one browser that is on its own too big for the + * computer is the limit's problem, and closing it would only start it again on the next request. + * + * Never one `spared` names, which is a Bot whose wheel a person holds or has been asked to take. + * Closing that browser closes it under somebody's hands, or under a page the Bot is waiting for + * somebody to come and work, and memory pressure is not a reason to do either. + */ +export function chooseForMemory( + running: Iterable<[string, Evictable]>, + goesLast: (botId: string) => boolean = () => false, + spared: (botId: string) => boolean = () => false, +): string | undefined { + const entries = [...running]; + return inClosingOrder(entries, newestOf(entries), goesLast) + .map(([botId]) => botId) + .find((botId) => !spared(botId)); } /** @@ -104,3 +149,40 @@ export function chooseIdle( .filter(([botId, entry]) => entry.usedAt <= cutoff && !stays(botId)) .map(([botId]) => botId); } + +/** Enough of a tab for the choice. Keeps this file independent of what else is on a page. */ +export type IdleTab = { + /** When the Bot or a person last acted on it, it navigated, or it was the page in front. */ + usedAt: number; + /** The page the Bot is on, which is the one the live screen shows and its next action lands in. */ + front: boolean; + /** A page that is never the Bot's to lose: an extension's own (the Recorder's) or DevTools. */ + kept?: boolean; +}; + +/** + * Which of a Bot's background tabs to close because nothing has touched them. + * + * A Bot is always on its newest page (`chooseLivePage`): a window a site opens is where its work + * went, and it has no way back to an older tab except that newer ones close. So a tab left behind + * (the page a sign-in popup was opened from, a link that opened in a new tab and was never closed) + * holds a renderer and its memory for nothing, for as long as the browser stays up. Closed past a + * timeout rather than at once, because an opener whose popup is still mid sign-in is waiting for + * that popup to come back to it. + * + * Never the page in front, which is the one the Bot is on, so closing tabs never moves it: it stays + * the newest open page, which is what the Bot follows. And never anything when which page is in + * front is not exactly one, because then any tab could be the one the Bot is on. That also means the + * last tab is never closed, since a single tab is the one in front. A timeout of zero or less + * switches this off. + */ +export function chooseIdleTabs( + tabs: readonly T[], + idleTimeoutMs: number, + now: number, +): T[] { + if (idleTimeoutMs <= 0) return []; + if (tabs.filter((tab) => tab.front).length !== 1) return []; + const cutoff = now - idleTimeoutMs; + return tabs.filter((tab) => !tab.front && !tab.kept && tab.usedAt <= cutoff); +} diff --git a/agent-computer/src/container-memory.ts b/agent-computer/src/container-memory.ts new file mode 100644 index 0000000000..fc2ee3c88b --- /dev/null +++ b/agent-computer/src/container-memory.ts @@ -0,0 +1,180 @@ +/** + * How much of its memory this computer is using, read the way Kubernetes reads it. + * + * WHY THIS EXISTS. A computer runs one Chromium per Bot that is browsing, and the only limits on them + * were a count and an idle timeout. Neither looks at memory, so a few heavy pages could push the + * container past its limit, and the kernel's OOM killer does not choose by use. On Kubernetes with + * cgroup v2 the kubelet sets `memory.oom.group` (since 1.28), so one OOM kills every process in the + * container and every Bot's browser on it goes at once; elsewhere the kernel kills the largest + * process, which is somebody's browser picked by size. Knowing how close the container is lets the + * computer close the least recently used browser first, which costs that Bot a relaunch and nothing + * else, because its profile is on disk. + * + * WORKING SET, NOT USAGE. The cgroup's usage counts the page cache, and most of a page cache is files + * the kernel will drop the moment anything needs the room: a container that wrote a large download + * reads as nearly full while it is nowhere near being killed. Kubernetes judges a container by its + * working set, which is usage less the inactive file cache, and that is what this reads, so the + * computer measures itself the way the kubelet measures it. cAdvisor computes it in + * `lib/container/libcontainer/handler.go`: `memory.current` less `inactive_file` on cgroup v2, and + * `memory.usage_in_bytes` less `total_inactive_file` on cgroup v1, clamped at zero. Microsoft's AKS + * guide to high memory consumption and the widely cited "Memory_working_set vs Memory_rss in + * Kubernetes" describe the same rule. + * + * THE LIMIT IS WHICHEVER RUNS OUT FIRST. A container started with no limit (`docker run` with no + * `--memory`, Docker Desktop's default) reports `max` on cgroup v2 and a number near 2^63 on cgroup + * v1. Inside a container, the machine's own memory is the VM's, and that is what actually runs out, + * so the limit is the smaller of the cgroup's and the machine's. + * + * NO CGROUP, NO READING. A computer that is not in a container (the one OpenBot Desktop runs on a + * person's Mac for their own browser, or one started by hand on a Linux desktop, whose view of + * `/sys/fs/cgroup` is the root, where these files do not exist) has nothing here to read, and closes + * nothing for memory. The count cap is still there. + * + * Its own file, with no Playwright import, for the reason `browser-eviction.ts` is: the parsing is + * the part with a wrong answer available, and the tests feed it the files real containers wrote. + */ + +import { readFile } from "node:fs/promises"; +import { totalmem } from "node:os"; +import { join } from "node:path"; + +/** How much memory the computer is using, and how much it may. Both in bytes. */ +export type MemoryReading = { workingSet: number; limit: number }; + +/** Read one cgroup file, or nothing when it is not there. Injected so a test can point it elsewhere. */ +export type ReadCgroupFile = (path: string) => Promise; + +/** Where a container sees its own cgroup, under cgroup v2 and the v1 memory controller. */ +export const CGROUP_ROOT = "/sys/fs/cgroup"; + +const readCgroupFile: ReadCgroupFile = (path) => + readFile(path, "utf8").catch(() => undefined); + +/** A whole number of bytes, as a cgroup file writes one. Anything else is not a reading. */ +function bytes(text: string | undefined): number | undefined { + const trimmed = text?.trim(); + if (!trimmed || !/^\d+$/.test(trimmed)) return undefined; + return Number(trimmed); +} + +/** One counter from a `memory.stat` file, whose lines are ` `. */ +export function statValue(stat: string, name: string): number | undefined { + for (const line of stat.split("\n")) { + const [key, value] = line.trim().split(/\s+/); + if (key === name) return bytes(value); + } + return undefined; +} + +/** + * Usage less the inactive file cache, clamped at zero, as cAdvisor does it. A `memory.stat` without + * the counter leaves usage as it is, also as cAdvisor does. + */ +function workingSetOf( + usage: number, + stat: string | undefined, + inactiveFile: string, +): number { + const inactive = + stat === undefined ? undefined : statValue(stat, inactiveFile); + if (inactive === undefined) return usage; + return usage < inactive ? 0 : usage - inactive; +} + +/** + * The limit that runs out first: the cgroup's, or the machine's when the cgroup has none or claims + * more than the machine has. Cgroup v2 writes `max` for none; v1 writes a number near 2^63, which + * the machine is always smaller than. + */ +function limitOf( + cgroupLimit: string | undefined, + machineBytes: number, +): number | undefined { + if (cgroupLimit?.trim() === "max") return machineBytes; + const limit = bytes(cgroupLimit); + if (limit === undefined || limit === 0) return undefined; + return Math.min(limit, machineBytes); +} + +/** + * The computer's memory from cgroup v2's `memory.current`, `memory.max` and `memory.stat`. + * + * Undefined when the usage or the limit cannot be read, which is a computer with no cgroup of its + * own and so nothing to act on. + */ +export function cgroupV2Memory( + files: { current?: string; max?: string; stat?: string }, + machineBytes: number, +): MemoryReading | undefined { + const usage = bytes(files.current); + const limit = limitOf(files.max, machineBytes); + if (usage === undefined || limit === undefined) return undefined; + return { + workingSet: workingSetOf(usage, files.stat, "inactive_file"), + limit, + }; +} + +/** + * The computer's memory from cgroup v1's `memory.usage_in_bytes`, `memory.limit_in_bytes` and + * `memory.stat`, for a host that still mounts the v1 memory controller. + */ +export function cgroupV1Memory( + files: { usage?: string; limit?: string; stat?: string }, + machineBytes: number, +): MemoryReading | undefined { + const usage = bytes(files.usage); + const limit = limitOf(files.limit, machineBytes); + if (usage === undefined || limit === undefined) return undefined; + return { + workingSet: workingSetOf(usage, files.stat, "total_inactive_file"), + limit, + }; +} + +/** + * Read the computer's memory now: cgroup v2 first, then v1, then nothing. + * + * Read again every time it is asked rather than cached, because the caller asks precisely when it + * has just closed a browser and wants to know whether that was enough. + */ +export async function readContainerMemory({ + root = CGROUP_ROOT, + read = readCgroupFile, + machineBytes = totalmem(), +}: { + root?: string; + read?: ReadCgroupFile; + machineBytes?: number; +} = {}): Promise { + const [current, max, stat] = await Promise.all([ + read(join(root, "memory.current")), + read(join(root, "memory.max")), + read(join(root, "memory.stat")), + ]); + if (current !== undefined) + return cgroupV2Memory({ current, max, stat }, machineBytes); + const v1 = join(root, "memory"); + const [usage, limit, v1Stat] = await Promise.all([ + read(join(v1, "memory.usage_in_bytes")), + read(join(v1, "memory.limit_in_bytes")), + read(join(v1, "memory.stat")), + ]); + if (usage === undefined) return undefined; + return cgroupV1Memory({ usage, limit, stat: v1Stat }, machineBytes); +} + +/** + * Whether the computer is close enough to its limit to start closing browsers. + * + * At or above the share, not only above it, so a deployment that set 85 has acted by the time it + * reads 85. A share of zero or less switches it off, as a timeout of zero switches off the idle + * sweep. + */ +export function memoryRunsHigh( + reading: MemoryReading, + highPercent: number, +): boolean { + if (highPercent <= 0) return false; + return reading.workingSet * 100 >= reading.limit * highPercent; +} diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index 62bad1fbb1..c741ff631a 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -310,7 +310,14 @@ const profiles = createProfiles( if (sessions.get(botId)) sessions.renewRun(botId); await sessions.get(botId)?.viewer.releaseAll(COMPUTER_STOPPED); }, - { personDisplay: PERSON_DISPLAY }, + { + personDisplay: PERSON_DISPLAY, + // A browser a person is working in, or has been asked to, is never closed for memory, and none + // of its tabs are closed for being idle. A person's own browser is not a Bot's wheel at all: it + // takes the cap's rules, a native window last and a cloud one by use. + heldByPerson: (botId) => + !isPersonBrowserId(botId) && sessions.personHasTheWheel(botId), + }, ); /** diff --git a/agent-computer/src/profiles.ts b/agent-computer/src/profiles.ts index 2cdf948d2f..816cfd47aa 100644 --- a/agent-computer/src/profiles.ts +++ b/agent-computer/src/profiles.ts @@ -38,8 +38,20 @@ import { join } from "node:path"; import { type BrowserContext, chromium, type Page } from "playwright"; import { isPersonBrowserId } from "../../shared/openbot-browser"; import { profileDirectoryFor } from "./bot-id"; -import { chooseEvictions, chooseIdle, keptOpen } from "./browser-eviction"; +import { + chooseEvictions, + chooseForMemory, + chooseIdle, + chooseIdleTabs, + type Evictable, + keptOpen, +} from "./browser-eviction"; import { type BrowserRuntime, browserRuntimeFromEnv } from "./browser-runtime"; +import { + type MemoryReading, + memoryRunsHigh, + readContainerMemory, +} from "./container-memory"; import { egressFor, egressLabel } from "./egress"; import { numberFromEnv, settleWithin } from "./env"; import { chooseLivePage } from "./live-page"; @@ -304,9 +316,52 @@ const IDLE_TIMEOUT_MS = numberFromEnv("COMPUTER_BROWSER_IDLE_MS", 30 * 60_000, { zeroSwitchesItOff: true, }); -/** How often the idle sweep looks. Cheap: it walks a map of at most `MAX_LIVE_BROWSERS`. */ +/** + * How often the idle sweep looks. Cheap: it walks a map of at most `MAX_LIVE_BROWSERS`, their tabs, + * and three small files under `/sys/fs/cgroup`. + */ const IDLE_SWEEP_MS = 60_000; +/** + * At what share of the computer's memory it starts closing browsers, least recently used first. + * + * The cap counts browsers, and a browser is not a fixed size: a few heavy pages can take a container + * past its memory limit well under the cap, and the OOM killer does not choose by use: on Kubernetes + * with cgroup v2 it kills every process in the container together, every Bot's browser at once, and + * nothing can reach the computer until it comes back. So memory is read the way Kubernetes reads it (`container-memory.ts`) + * before every launch, which is when memory jumps, and on every tick of the idle sweep, and while it + * is at or above this share of the limit the least recently used browser is closed and memory read + * again. Closing one costs its Bot a relaunch and nothing else, the same as the cap. + * + * Eighty-five leaves room for the launch that asked, and for a page that grows after the check, + * before the limit is reached. Zero switches it off, as it does for the idle sweep. A computer with no + * cgroup of its own (OpenBot Desktop's, on a person's Mac) has no limit to read and closes nothing + * for memory either way. + */ +const MEMORY_HIGH_PERCENT = numberFromEnv("COMPUTER_MEMORY_HIGH_PERCENT", 85, { + zeroSwitchesItOff: true, + max: 100, +}); + +/** + * How long a Bot's background tab may sit untouched before the idle sweep closes it. + * + * A Bot is always on its newest page, so a tab it left behind (the page a sign-in popup opened from, + * a link a site opened in a new tab) is one it cannot get back to, holding a renderer and its memory + * for as long as the browser is up. Ten minutes is long enough for a popup's sign-in to come back to + * the page that opened it, and a third of the thirty after which a whole idle browser is closed, so a + * tab left behind in a browser that is still in use does not wait for the browser to go. See + * `chooseIdleTabs` for what is never closed. Zero switches it off. + * + * No count of tabs alongside it. A count is the wrong measure for the same reason the browser cap is + * not enough on its own: one heavy tab outweighs twenty light ones, and memory is now watched + * directly. A count would also close tabs a site opened a moment ago, in the middle of the flow they + * were opened for, where a timeout only ever closes what has been left alone. + */ +const TAB_IDLE_MS = numberFromEnv("COMPUTER_TAB_IDLE_MS", 10 * 60_000, { + zeroSwitchesItOff: true, +}); + /** * Told whenever a Bot's browser is closed and forgotten, before the close is waited on. * @@ -337,6 +392,21 @@ export type ProfilesOptions = { * as it is now. */ personDisplay?: PersonDisplay; + /** + * The computer's memory now, or nothing when it has no limit to read. The container's own cgroup + * unless a test hands in the reading it wants the computer to see. + */ + memory?: () => Promise; + /** {@link MEMORY_HIGH_PERCENT}, for a test. */ + memoryHighPercent?: number; + /** {@link TAB_IDLE_MS}, for a test that cannot wait ten minutes. */ + tabIdleMs?: number; + /** + * Whether a person holds this Bot's wheel, or has been asked to take it. Such a browser is never + * closed for memory, and none of its tabs are closed for being idle: a person may be working across + * them. The computer's sessions answer it (`sessions.personHasTheWheel`); without it, nobody does. + */ + heldByPerson?: (botId: string) => boolean; }; /** Opening a person's browser, which is the one way it starts. See `person-browser.ts`. */ @@ -366,6 +436,22 @@ export function createProfiles( DISPLAY: process.env.DISPLAY, WAYLAND_DISPLAY: process.env.WAYLAND_DISPLAY, }; + const readMemory = options.memory ?? (() => readContainerMemory()); + const memoryHighPercent = options.memoryHighPercent ?? MEMORY_HIGH_PERCENT; + const tabIdleMs = options.tabIdleMs ?? TAB_IDLE_MS; + const heldByPerson = options.heldByPerson ?? (() => false); + /* + * A wheel that cannot be read counts as held. The control state throws when its store has failed, + * and the answer decides whether a browser or a tab is closed: keeping one is the mistake that + * costs nothing, and a throw here must never fail the launch that asked. + */ + const spared = (botId: string): boolean => { + try { + return heldByPerson(botId); + } catch { + return true; + } + }; type LiveBrowser = { context: BrowserContext; @@ -400,6 +486,26 @@ export function createProfiles( const live = new Map(); /** A person's native window: the idle sweep leaves it and the cap takes it last. See `keptOpen`. */ const stays = keptOpen(personWindow(runtime)); + /** + * When each of a Bot's tabs was last used: acted on, navigated, or in front. Decides which + * background tabs the idle sweep closes. Weak, so a closed page takes its entry with it. + */ + const tabUsedAt = new WeakMap(); + const touchTab = (page: Page): void => { + tabUsedAt.set(page, Date.now()); + }; + /** A tab never seen before counts as used now, so one this process missed is not closed at once. */ + const tabLastUsed = (page: Page): number => { + const at = tabUsedAt.get(page) ?? Date.now(); + tabUsedAt.set(page, at); + return at; + }; + /** A tab that navigates is in use, a background one included: a site moving it on, a sign-in finishing. */ + const touchOnNavigation = (page: Page): void => { + page.on("framenavigated", (frame) => { + if (frame === page.mainFrame()) touchTab(page); + }); + }; /** Launches in flight, so a cold computer is started once however many callers ask at once. */ const starting = new Map>(); /** Closes and resets in flight, so a Bot's browser is never reopened from a profile being closed or deleted. */ @@ -471,10 +577,91 @@ export function createProfiles( }; /** - * Close browsers nothing has touched for a while. + * Close browsers, least recently used first, while the computer's memory runs high. + * + * One at a time, reading memory again after each close, because what one browser was holding is + * not known until it has gone. `asking` is the Bot whose launch this runs ahead of: counted as the + * newest, so it is never what makes room for itself, while every other browser here can be. Stops + * when memory is back under the line, when nothing is left that may be closed (`chooseForMemory`), + * or when the same browser comes up twice, which a close racing this one could otherwise loop on. + */ + const relieveMemory = async (asking?: string): Promise => { + if (memoryHighPercent <= 0) return; + const tried = new Set(); + for (;;) { + // A reading that fails is a computer with nothing to act on, the same as one with no cgroup. + const reading = await readMemory().catch(() => undefined); + if (!reading || !memoryRunsHigh(reading, memoryHighPercent)) return; + const running: [string, Evictable][] = [...live.entries()]; + if (asking && !live.has(asking)) + running.push([asking, { usedAt: Date.now() }]); + const next = chooseForMemory(running, stays, spared); + if (!next || tried.has(next)) return; + tried.add(next); + console.info( + JSON.stringify({ + type: "computer-browser-closed-for-memory", + botId: next, + workingSetBytes: reading.workingSet, + limitBytes: reading.limit, + }), + ); + await evict(next, "the computer's memory ran high"); + } + }; + + /** + * Close the background tabs of every Bot's browser that nothing has touched for a while. + * + * Never a person's browser, whose tabs are theirs and which has no Bot in it, and never a Bot's + * while a person holds its wheel, because they may be working across its tabs. Within a Bot's + * browser, `chooseIdleTabs` decides, and the page the Bot is on is never one of them, so it is + * still the newest open page afterwards and the record does not move. The address is not logged: + * a page left behind can be a sign-in link with a one-time token in it. + */ + const closeIdleTabs = async (): Promise => { + if (tabIdleMs <= 0) return 0; + const now = Date.now(); + let closed = 0; + for (const [botId, running] of [...live.entries()]) { + if (running.person || isPersonBrowserId(botId) || spared(botId)) continue; + // Closed while an earlier Bot's tabs were being closed: nothing of it is left to sweep. + if (live.get(botId) !== running) continue; + running.retarget(); + const tabs = running.context + .pages() + .filter((page) => !page.isClosed()) + .map((page) => ({ + page, + usedAt: tabLastUsed(page), + front: page === running.page, + kept: + page.url().startsWith("chrome-extension://") || + page.url().startsWith("devtools://"), + })); + for (const { page } of chooseIdleTabs(tabs, tabIdleMs, now)) { + console.info( + JSON.stringify({ + type: "computer-tab-closed", + botId, + reason: "it had been idle", + }), + ); + await page.close().catch(() => undefined); + closed += 1; + } + } + return closed; + }; + + /** + * Close browsers nothing has touched for a while, then the tabs Bots left behind, then, if memory + * still runs high, browsers by use. * * The cap answers concurrent breadth; this answers a Bot used once last Tuesday whose browser is - * still resident and doing nothing for anybody. + * still resident and doing nothing for anybody. In that order because each step is gentler than the + * next: a browser nobody is using goes before a tab of one somebody is, and a tab nobody can reach + * goes before a browser that is still in use. */ const sweepIdle = async (): Promise => { for (const botId of chooseIdle( @@ -485,6 +672,12 @@ export function createProfiles( )) { await evict(botId, "it had been idle"); } + const tabsClosed = await closeIdleTabs().catch(() => 0); + // A closed tab's renderer exits a moment after the tab does, so memory read at once would still + // count it, and a browser could be closed for memory the tabs had already given back. + if (tabsClosed > 0) + await new Promise((resolve) => setTimeout(resolve, CLOSE_SETTLE_MS)); + await relieveMemory(); }; const idleSweep = setInterval(() => { @@ -723,8 +916,10 @@ export function createProfiles( existing?.context.browser()?.isConnected() && !existing.page.isClosed() ) { - // Touched on every use, which is what makes "least recently used" mean anything. + // Touched on every use, which is what makes "least recently used" mean anything. The page in + // front with it: a Bot's next action, and a person's input on the live screen, land there. existing.usedAt = Date.now(); + touchTab(existing.page); return existing.page; } if (existing) { @@ -748,6 +943,9 @@ export function createProfiles( if (isPersonBrowserId(botId) && !open) throw new BrowserNotOpenError(); const launch = (async () => { + // Before the launch, which is when memory jumps: a cold Chromium and its first page are the + // largest single step up a computer takes, and the OOM killer does not wait for the cap. + await relieveMemory(botId); const dir = directoryFor(botId); // A second native helper can target the same data root on another port. Do not remove an // active Chrome profile's lock; Chrome reports contention and handles its own stale locks. @@ -804,9 +1002,14 @@ export function createProfiles( retarget: () => {}, ...(sessionCookies ? { sessionCookies } : {}), }; + touchTab(page); + touchOnNavigation(page); record.retarget = () => { const next = chooseLivePage(context.pages()); if (!next || next === record.page) return; + // Both are in use at this moment: the one going behind was in front until now. + touchTab(record.page); + touchTab(next); record.page = next; console.info( JSON.stringify({ @@ -828,6 +1031,8 @@ export function createProfiles( // in a new window is neither shown to the person taking the wheel nor reachable by input. context.on("page", (opened) => { if (!local) void attachPageNetwork(context, opened, network); + touchTab(opened); + touchOnNavigation(opened); record.retarget(); // A popup closes itself when it succeeds, and the record must move back to the opener // rather than leave a closed page to be read as a dead browser. diff --git a/agent-computer/src/sessions.ts b/agent-computer/src/sessions.ts index 72177328ab..0b59f39813 100644 --- a/agent-computer/src/sessions.ts +++ b/agent-computer/src/sessions.ts @@ -102,15 +102,24 @@ export function createSessions(options: SessionsOptions) { for (const [botId, session] of [...sessions.entries()]) { if (session.viewer.occupied()) continue; if (options.isLive(botId)) continue; - if ( - session.control.get().request?.status === "waiting" || - session.control.get().holder === "human" - ) - continue; + if (wheelWithPerson(session)) continue; sessions.delete(botId); } } + /** + * Whether a person holds this Bot's wheel, or the Bot has asked one to take it and is waiting. + * + * Either way the browser is somebody's to work in or about to be, so it is kept: here, when the map + * is trimmed, and by the computer when it closes browsers for memory or closes a Bot's idle tabs. + * A request that has waited past its time has expired by the time this asks (`get` expires it), so + * a Bot that asked and was never answered does not hold its browser for ever. + */ + function wheelWithPerson(session: BotSession): boolean { + const state = session.control.get(); + return state.request?.status === "waiting" || state.holder === "human"; + } + function sessionFor(botId: string): BotSession { const existing = sessions.get(botId); if (existing) return existing; @@ -164,6 +173,17 @@ export function createSessions(options: SessionsOptions) { return sessions.get(botId); }, + /** + * Whether a person holds this Bot's wheel or has been asked to take it (`wheelWithPerson`). + * + * Asks without answering, like `get`: a Bot with no session has nobody at its wheel, and this is + * asked on the paths that close browsers, where creating an entry would grow the map. + */ + personHasTheWheel(botId: string): boolean { + const session = sessions.get(botId); + return session ? wheelWithPerson(session) : false; + }, + /** * Start a new run for this Bot, because its browser session is over. * diff --git a/agent-computer/tests/browser-eviction.test.ts b/agent-computer/tests/browser-eviction.test.ts index 85f6f8ae36..3c5f9e3ae6 100644 --- a/agent-computer/tests/browser-eviction.test.ts +++ b/agent-computer/tests/browser-eviction.test.ts @@ -1,5 +1,11 @@ import { describe, expect, test } from "bun:test"; -import { chooseEvictions, chooseIdle, keptOpen } from "../src/browser-eviction"; +import { + chooseEvictions, + chooseForMemory, + chooseIdle, + chooseIdleTabs, + keptOpen, +} from "../src/browser-eviction"; import { numberFromEnv } from "../src/env"; /** @@ -252,3 +258,142 @@ describe("a person's cloud browser among Bots' browsers", () => { ]); }); }); + +/** + * Which browser to close next when the computer's memory runs high. + * + * The cap counts browsers and the idle sweep counts minutes, and neither looks at memory, so a few + * heavy pages took a container past its limit and the OOM killer took every browser on it at once. + * The computer now closes browsers one at a time while memory runs high, reading it again after each. + */ +describe("closing a browser because memory runs high", () => { + const PERSON = "person-0123456789abcdef0123456789abcdef"; + const isPerson = (botId: string) => botId.startsWith("person-"); + + test("closes the least recently used, by the cap's own order", () => { + expect(chooseForMemory(running(500, 100, 300, 900))).toBe("bot-1"); + }); + + test("never the newest, which before a launch is the Bot asking", () => { + // The caller adds the Bot whose browser is about to launch as the most recently used, so it is + // never what makes room for itself, however everything else is ranked. + const before = running(100, 200); + before.set("asking", { usedAt: 300 }); + const chosen: string[] = []; + for (;;) { + const next = chooseForMemory(before); + if (!next) break; + chosen.push(next); + before.delete(next); + } + expect(chosen).toEqual(["bot-0", "bot-1"]); + expect([...before.keys()]).toEqual(["asking"]); + }); + + test("never the last browser left: one too big on its own is the limit's problem", () => { + expect(chooseForMemory(running(100))).toBeUndefined(); + expect(chooseForMemory(running())).toBeUndefined(); + }); + + test("never a browser whose wheel a person holds", () => { + // bot-0 is the least recently used and a person is working in it: the next one goes instead. + const held = (botId: string) => botId === "bot-0"; + expect(chooseForMemory(running(100, 200, 300), undefined, held)).toBe( + "bot-1", + ); + // With only the held one and the newest left, nothing is closed. + expect(chooseForMemory(running(100, 300), undefined, held)).toBeUndefined(); + }); + + test("a person's native window goes after every Bot's browser, as at the cap", () => { + const withPerson = new Map([ + [PERSON, { usedAt: 1 }], + ...running(500, 300, 900), + ]); + expect(chooseForMemory(withPerson, isPerson)).toBe("bot-1"); + expect( + chooseForMemory( + new Map([[PERSON, { usedAt: 1 }], ...running(900)]), + isPerson, + ), + ).toBe(PERSON); + }); +}); + +/** + * Which of a Bot's background tabs to close because nobody has touched them. + * + * A Bot is always on its newest page, so a tab it left behind, the page a sign-in popup opened from + * or a link a site opened in a new tab, is one it cannot go back to, and it held a renderer for as + * long as the browser stayed up. + */ +describe("closing a Bot's idle background tabs", () => { + const NOW = 1_000_000; + const IDLE = 10 * 60_000; + const tab = (name: string, usedAt: number, front = false, kept = false) => ({ + name, + usedAt, + front, + kept, + }); + const names = (tabs: { name: string }[]) => tabs.map((t) => t.name); + + test("closes a background tab past the timeout, and keeps the page in front", () => { + const tabs = [ + tab("left behind", NOW - IDLE - 1), + tab("in front", NOW - IDLE - 1, true), + ]; + // The page in front is as old as the other and is still never closed: it is the one the Bot is on. + expect(names(chooseIdleTabs(tabs, IDLE, NOW))).toEqual(["left behind"]); + }); + + test("keeps a background tab used inside the timeout, and closes one at it", () => { + expect( + names( + chooseIdleTabs( + [ + tab("recent", NOW - IDLE + 1), + tab("at the line", NOW - IDLE), + tab("in front", NOW, true), + ], + IDLE, + NOW, + ), + ), + ).toEqual(["at the line"]); + }); + + test("never the last tab", () => { + expect(chooseIdleTabs([tab("only", 0, true)], IDLE, NOW)).toEqual([]); + }); + + test("closes nothing when which page is in front is not known", () => { + // Any tab could then be the one the Bot is on, and closing it would move the Bot. + expect(chooseIdleTabs([tab("a", 0), tab("b", 0)], IDLE, NOW)).toEqual([]); + expect( + chooseIdleTabs([tab("a", 0, true), tab("b", 0, true)], IDLE, NOW), + ).toEqual([]); + }); + + test("never an extension's own page or DevTools", () => { + expect( + names( + chooseIdleTabs( + [ + tab("recorder", 0, false, true), + tab("old", 0), + tab("front", NOW, true), + ], + IDLE, + NOW, + ), + ), + ).toEqual(["old"]); + }); + + test("a timeout of zero switches it off", () => { + expect( + chooseIdleTabs([tab("old", 0), tab("front", NOW, true)], 0, NOW), + ).toEqual([]); + }); +}); diff --git a/agent-computer/tests/browser-memory.test.ts b/agent-computer/tests/browser-memory.test.ts new file mode 100644 index 0000000000..50668eb623 --- /dev/null +++ b/agent-computer/tests/browser-memory.test.ts @@ -0,0 +1,232 @@ +import { afterAll, afterEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { MemoryReading } from "../src/container-memory"; + +/** + * A computer that closes browsers before its memory runs out, and a Bot's tabs it left behind. + * + * The cap counted browsers and the idle sweep counted minutes, and neither looked at memory, so a few + * heavy pages could take a container past its limit, where the OOM killer takes every browser on it + * at once rather than one. These drive `createProfiles` with real Chromium, handing it the memory + * reading a container would give, because the closes they cover are made from inside another Bot's + * launch and from the idle sweep, and no request reaches either. + * + * Asked for by name, like the other tests that launch a real Chromium: + * + * cd agent-computer && bunx playwright install chromium + * OPENBOT_COMPUTER_BROWSER=1 COMPUTER_BROWSER_MODE=headless bun test tests/browser-memory.test.ts + */ +const asked = process.env.OPENBOT_COMPUTER_BROWSER === "1"; +const LAUNCH_TIMEOUT_MS = 120_000; + +const html = (title: string) => + new Response(`${title}${title}`, { + headers: { "content-type": "text/html; charset=utf-8" }, + }); +const site = Bun.serve({ + port: 0, + fetch(request) { + return html(new URL(request.url).pathname.slice(1) || "home"); + }, +}); +const origin = `http://127.0.0.1:${site.port}`; + +const roots: string[] = []; +const said = spyOn(console, "info"); + +afterEach(() => { + said.mockClear(); +}); + +afterAll(async () => { + site.stop(true); + said.mockRestore(); + await Promise.all( + roots.map((root) => rm(root, { recursive: true, force: true })), + ); +}); + +/** The structured lines the computer logged of one type. */ +function logged(type: string): Record[] { + return said.mock.calls + .map(([line]) => { + try { + return JSON.parse(String(line)) as Record; + } catch { + return undefined; + } + }) + .filter((line): line is Record => line?.type === type); +} + +async function profilesFor( + name: string, + options: Parameters[2], +) { + const { createProfiles } = await import("../src/profiles"); + const root = await mkdtemp(join(tmpdir(), `openbot-${name}-`)); + roots.push(root); + const told: string[] = []; + const profiles = createProfiles( + root, + (botId) => { + told.push(botId); + }, + options, + ); + return { profiles, told }; +} + +/** + * A container where each running browser holds the same 450 MiB of a 1000 MiB limit, read from how + * many are running now, as the cgroup would read it. Two browsers are 90% (high at 85), one is 45%. + */ +const MIB = 1024 ** 2; +const LIMIT = 1000 * MIB; +const PER_BROWSER = 450 * MIB; +const perBrowser = + (running: () => number) => async (): Promise => ({ + workingSet: running() * PER_BROWSER, + limit: LIMIT, + }); + +describe.skipIf(!asked)("closing browsers when memory runs high", () => { + test( + "a launch on a computer whose memory runs high closes the least recently used first, never the one launching", + async () => { + let profiles: Awaited>["profiles"]; + const made = await profilesFor("memory", { + memory: perBrowser(() => profiles.liveCount()), + memoryHighPercent: 85, + }); + profiles = made.profiles; + try { + // One browser is 45%: the second launches without anything being closed. + await profiles.page("first"); + await profiles.page("second"); + expect(made.told).toEqual([]); + // The first is used again, so the second is now the least recently used: by use, not by + // which started first. A moment later, because use is kept to the millisecond and the + // second's launch can finish inside the same one. + await new Promise((resolve) => setTimeout(resolve, 5)); + await profiles.page("first"); + + // Two browsers are 90%. The third Bot's launch closes the second, reads 45%, and launches. + const third = await profiles.page("third"); + + expect(made.told).toEqual(["second"]); + expect(profiles.isLive("first")).toBe(true); + expect(profiles.isLive("second")).toBe(false); + expect(profiles.isLive("third")).toBe(true); + expect(third.isClosed()).toBe(false); + expect(logged("computer-browser-closed-for-memory")).toEqual([ + { + type: "computer-browser-closed-for-memory", + botId: "second", + workingSetBytes: 2 * PER_BROWSER, + limitBytes: LIMIT, + }, + ]); + } finally { + await made.profiles.closeAll(); + } + }, + LAUNCH_TIMEOUT_MS, + ); + + test( + "a browser a person is working in stays, and memory that will not come down never closes the last browser", + async () => { + // Memory that reads high whatever is closed: one browser too big on its own. The loop has to + // stop at the newest, not close its way to nothing. + const made = await profilesFor("memory-held", { + memory: async () => ({ workingSet: LIMIT, limit: LIMIT }), + memoryHighPercent: 85, + heldByPerson: (botId) => botId === "held", + }); + const { profiles } = made; + try { + await profiles.page("held"); + await profiles.page("other"); + // Both are live before the third arrives: launching "other" closed nothing it could, because + // the only other browser was the one a person holds. + expect(made.told).toEqual([]); + + await profiles.page("asking"); + // "other" goes; "held" is a person's, and "asking" is the launch itself. + expect(made.told).toEqual(["other"]); + expect(profiles.isLive("held")).toBe(true); + expect(profiles.isLive("asking")).toBe(true); + + // The idle sweep's tick reads the same and closes nothing more: what is left is held, or the + // newest. + await profiles.sweepIdleNow(); + expect(made.told).toEqual(["other"]); + expect(profiles.liveCount()).toBe(2); + } finally { + await profiles.closeAll(); + } + }, + LAUNCH_TIMEOUT_MS, + ); +}); + +describe.skipIf(!asked)("closing a Bot's idle background tabs", () => { + test( + "the sweep closes a background tab left past the timeout, and the Bot stays on its page", + async () => { + let held = true; + const made = await profilesFor("tabs", { + memory: async () => undefined, + // The shortest timeout the sweep acts on, so the test waits milliseconds rather than ten minutes. + tabIdleMs: 1, + heldByPerson: () => held, + }); + const { profiles } = made; + try { + const behind = await profiles.page("tabs"); + await behind.goto(`${origin}/left-behind`, { + waitUntil: "domcontentloaded", + }); + const front = await behind.context().newPage(); + await front.goto(`${origin}/in-front`, { + waitUntil: "domcontentloaded", + }); + // The new tab is the Bot's page now, as a window a site opens would be. + expect(await profiles.page("tabs")).toBe(front); + await new Promise((resolve) => setTimeout(resolve, 25)); + + // A person holds the wheel: they may be working across these tabs, so nothing is closed. + await profiles.sweepIdleNow(); + expect(behind.isClosed()).toBe(false); + + held = false; + await new Promise((resolve) => setTimeout(resolve, 25)); + said.mockClear(); + await profiles.sweepIdleNow(); + + expect(behind.isClosed()).toBe(true); + expect(front.isClosed()).toBe(false); + expect(front.context().pages()).toEqual([front]); + expect(logged("computer-tab-closed")).toEqual([ + { + type: "computer-tab-closed", + botId: "tabs", + reason: "it had been idle", + }, + ]); + // Still the Bot's page: the record never moved, and the browser was not closed. + expect(logged("computer-page-changed")).toEqual([]); + const after = await profiles.page("tabs"); + expect(after).toBe(front); + expect(after.url()).toBe(`${origin}/in-front`); + expect(made.told).toEqual([]); + } finally { + await profiles.closeAll(); + } + }, + LAUNCH_TIMEOUT_MS, + ); +}); diff --git a/agent-computer/tests/container-memory.test.ts b/agent-computer/tests/container-memory.test.ts new file mode 100644 index 0000000000..4e8827351c --- /dev/null +++ b/agent-computer/tests/container-memory.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { memoryRunsHigh, readContainerMemory } from "../src/container-memory"; + +/** + * How much of its memory a computer is using, from the files a real container wrote. + * + * Every fixture under `fixtures/cgroup` is laid out the way a container sees `/sys/fs/cgroup`, and + * read here with the same reader the computer uses, only rooted elsewhere. The cgroup v2 ones were + * captured from throwaway containers (ubuntu:24.04 on Docker 29.5.2, cgroup v2): + * + * v2-limit-512m docker run --rm --memory 512m, at rest + * v2-no-limit docker run --rm with no --memory, so `memory.max` reads `max` + * v2-page-cache --memory 256m after writing 96 MB to a file, which sits in the page cache + * v2-shm-near-limit --memory 256m --shm-size 256m after writing 224 MB to /dev/shm, which is + * where Chromium keeps the memory its processes share + * + * `v1-no-limit/memory/memory.stat` is a cgroup v1 file captured from a container with no limit, as + * Apache Druid keeps it for its own tests (processing/src/test/resources/memory.stat); this machine's + * kernel runs cgroup v2, which cannot mount the v1 memory controller. Its `memory.limit_in_bytes` is + * the `hierarchical_memory_limit` that same file reports, and its `memory.usage_in_bytes` is the + * file's `cache` plus `rss`, which is what v1 usage is made of. + */ + +const FIXTURES = join(import.meta.dir, "fixtures", "cgroup"); + +/** The memory of the VM the v2 fixtures were captured in: `MemTotal` from its /proc/meminfo. */ +const VM_BYTES = 16_353_252 * 1024; + +const read = (fixture: string) => + readContainerMemory({ + root: join(FIXTURES, fixture), + machineBytes: VM_BYTES, + }); + +describe("reading a container's memory as Kubernetes reads it", () => { + test("a container with a limit is judged against that limit", async () => { + // `memory.current` less `inactive_file`, exactly as cAdvisor computes the working set. + expect(await read("v2-limit-512m")).toEqual({ + workingSet: 1_228_800 - 16_384, + limit: 512 * 1024 * 1024, + }); + }); + + test("a container with no limit is judged against the machine, which is what runs out", async () => { + // `memory.max` reads `max`. Inside a container the machine is the VM, and its memory is the + // limit the OOM killer actually enforces. + expect(await read("v2-no-limit")).toEqual({ + workingSet: 1_568_768, + limit: VM_BYTES, + }); + }); + + test("a file in the page cache is not memory the container is short of", async () => { + /* + * The case that makes it the working set rather than usage. After writing a 96 MB file, usage + * reads 39% of a 256 MB limit, and nearly all of it is inactive file cache the kernel drops the + * moment anything needs the room. Judged on usage, a Bot saving a large download would close + * other Bots' browsers for memory nobody was short of. + */ + const reading = await read("v2-page-cache"); + expect(reading).toEqual({ + workingSet: 105_312_256 - 100_982_784, + limit: 256 * 1024 * 1024, + }); + if (!reading) throw new Error("no reading"); + expect(105_312_256 / reading.limit).toBeGreaterThan(0.39); + expect(memoryRunsHigh(reading, 30)).toBe(false); + }); + + test("memory held in /dev/shm counts, and near the limit it reads as high", async () => { + // Shared memory cannot be dropped like a cache, and Chromium's processes share rendered frames + // through it. 224 MB of it is 88% of a 256 MB limit. + const reading = await read("v2-shm-near-limit"); + expect(reading).toEqual({ + workingSet: 236_740_608, + limit: 256 * 1024 * 1024, + }); + if (!reading) throw new Error("no reading"); + expect(memoryRunsHigh(reading, 85)).toBe(true); + expect(memoryRunsHigh(reading, 90)).toBe(false); + }); + + test("cgroup v1, with no limit, is read from its own files and judged against the machine", async () => { + // v1 writes no limit as a number near 2^63 rather than `max`, and its inactive cache is + // `total_inactive_file`, which includes the cgroups beneath it. + expect(await read("v1-no-limit")).toEqual({ + workingSet: 9_711_616 - 2_019_328, + limit: VM_BYTES, + }); + }); + + test("a computer with no cgroup of its own reads nothing, and so closes nothing for memory", async () => { + // OpenBot Desktop's computer on a Mac, or one started by hand on a Linux desktop, whose view of + // /sys/fs/cgroup is the root, where none of these files exist. + const empty = await mkdtemp(join(tmpdir(), "openbot-no-cgroup-")); + try { + expect( + await readContainerMemory({ root: empty, machineBytes: VM_BYTES }), + ).toBeUndefined(); + } finally { + await rm(empty, { recursive: true, force: true }); + } + }); +}); + +describe("whether memory runs high", () => { + const LIMIT = 1_000; + + test("at the share it acts, and a byte under it does not", () => { + // At or above, so a deployment that said 85 has acted by the time it reads 85. + expect(memoryRunsHigh({ workingSet: 850, limit: LIMIT }, 85)).toBe(true); + expect(memoryRunsHigh({ workingSet: 849, limit: LIMIT }, 85)).toBe(false); + }); + + test("a share of zero switches it off", () => { + expect(memoryRunsHigh({ workingSet: LIMIT, limit: LIMIT }, 0)).toBe(false); + }); +}); diff --git a/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.limit_in_bytes b/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.limit_in_bytes new file mode 100644 index 0000000000..564113cfaf --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.limit_in_bytes @@ -0,0 +1 @@ +9223372036854771712 diff --git a/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.stat b/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.stat new file mode 100644 index 0000000000..e68ddceedf --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.stat @@ -0,0 +1,36 @@ +cache 7892992 +rss 1818624 +rss_huge 0 +shmem 0 +mapped_file 1327104 +dirty 0 +writeback 0 +swap 0 +pgpgin 8346 +pgpgout 5975 +pgfault 13137 +pgmajfault 120 +inactive_anon 0 +active_anon 1757184 +inactive_file 2019328 +active_file 5873664 +unevictable 0 +hierarchical_memory_limit 9223372036854771712 +hierarchical_memsw_limit 9223372036854771712 +total_cache 7892992 +total_rss 1818624 +total_rss_huge 0 +total_shmem 0 +total_mapped_file 1327104 +total_dirty 0 +total_writeback 0 +total_swap 0 +total_pgpgin 8346 +total_pgpgout 5975 +total_pgfault 13137 +total_pgmajfault 120 +total_inactive_anon 0 +total_active_anon 1757184 +total_inactive_file 2019328 +total_active_file 5873664 +total_unevictable 0 diff --git a/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.usage_in_bytes b/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.usage_in_bytes new file mode 100644 index 0000000000..69f9852b49 --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v1-no-limit/memory/memory.usage_in_bytes @@ -0,0 +1 @@ +9711616 diff --git a/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.current b/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.current new file mode 100644 index 0000000000..9ff0b8de94 --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.current @@ -0,0 +1 @@ +1228800 diff --git a/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.max b/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.max new file mode 100644 index 0000000000..1f5856896e --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.max @@ -0,0 +1 @@ +536870912 diff --git a/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.stat b/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.stat new file mode 100644 index 0000000000..a6a5c98b86 --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-limit-512m/memory.stat @@ -0,0 +1,61 @@ +anon 188416 +file 77824 +kernel 1069056 +kernel_stack 32768 +pagetables 86016 +sec_pagetables 0 +percpu 43920 +sock 0 +vmalloc 8192 +shmem 0 +zswap 0 +zswapped 0 +file_mapped 28672 +file_dirty 0 +file_writeback 0 +swapcached 0 +anon_thp 0 +file_thp 0 +shmem_thp 0 +inactive_anon 0 +active_anon 122880 +inactive_file 16384 +active_file 53248 +unevictable 0 +slab_reclaimable 95992 +slab_unreclaimable 391144 +slab 487136 +workingset_refault_anon 0 +workingset_refault_file 0 +workingset_activate_anon 0 +workingset_activate_file 0 +workingset_restore_anon 0 +workingset_restore_file 0 +workingset_nodereclaim 0 +pgdemote_kswapd 0 +pgdemote_direct 0 +pgdemote_khugepaged 0 +pgscan 0 +pgsteal 0 +pgscan_kswapd 0 +pgscan_direct 0 +pgscan_khugepaged 0 +pgsteal_kswapd 0 +pgsteal_direct 0 +pgsteal_khugepaged 0 +pgfault 1473 +pgmajfault 2 +pgrefill 0 +pgactivate 0 +pgdeactivate 0 +pglazyfree 0 +pglazyfreed 0 +swpin_zero 0 +swpout_zero 0 +zswpin 0 +zswpout 0 +zswpwb 0 +thp_fault_alloc 0 +thp_collapse_alloc 0 +thp_swpout 0 +thp_swpout_fallback 0 diff --git a/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.current b/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.current new file mode 100644 index 0000000000..66fb7c4c10 --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.current @@ -0,0 +1 @@ +1568768 diff --git a/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.max b/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.max new file mode 100644 index 0000000000..355295a05a --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.max @@ -0,0 +1 @@ +max diff --git a/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.stat b/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.stat new file mode 100644 index 0000000000..c2f0ae556e --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-no-limit/memory.stat @@ -0,0 +1,61 @@ +anon 188416 +file 0 +kernel 1073152 +kernel_stack 32768 +pagetables 86016 +sec_pagetables 0 +percpu 44560 +sock 0 +vmalloc 8192 +shmem 0 +zswap 0 +zswapped 0 +file_mapped 0 +file_dirty 0 +file_writeback 0 +swapcached 0 +anon_thp 0 +file_thp 0 +shmem_thp 0 +inactive_anon 0 +active_anon 155648 +inactive_file 0 +active_file 0 +unevictable 0 +slab_reclaimable 90760 +slab_unreclaimable 393544 +slab 484304 +workingset_refault_anon 0 +workingset_refault_file 0 +workingset_activate_anon 0 +workingset_activate_file 0 +workingset_restore_anon 0 +workingset_restore_file 0 +workingset_nodereclaim 0 +pgdemote_kswapd 0 +pgdemote_direct 0 +pgdemote_khugepaged 0 +pgscan 0 +pgsteal 0 +pgscan_kswapd 0 +pgscan_direct 0 +pgscan_khugepaged 0 +pgsteal_kswapd 0 +pgsteal_direct 0 +pgsteal_khugepaged 0 +pgfault 1469 +pgmajfault 0 +pgrefill 0 +pgactivate 0 +pgdeactivate 0 +pglazyfree 0 +pglazyfreed 0 +swpin_zero 0 +swpout_zero 0 +zswpin 0 +zswpout 0 +zswpwb 0 +thp_fault_alloc 0 +thp_collapse_alloc 0 +thp_swpout 0 +thp_swpout_fallback 0 diff --git a/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.current b/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.current new file mode 100644 index 0000000000..67b2df57ce --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.current @@ -0,0 +1 @@ +105312256 diff --git a/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.max b/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.max new file mode 100644 index 0000000000..853f47e70f --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.max @@ -0,0 +1 @@ +268435456 diff --git a/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.stat b/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.stat new file mode 100644 index 0000000000..f0279ac447 --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-page-cache/memory.stat @@ -0,0 +1,61 @@ +anon 184320 +file 101089280 +kernel 3846144 +kernel_stack 32768 +pagetables 81920 +sec_pagetables 0 +percpu 43920 +sock 0 +vmalloc 8192 +shmem 0 +zswap 0 +zswapped 0 +file_mapped 0 +file_dirty 0 +file_writeback 0 +swapcached 0 +anon_thp 0 +file_thp 0 +shmem_thp 0 +inactive_anon 0 +active_anon 167936 +inactive_file 100982784 +active_file 106496 +unevictable 0 +slab_reclaimable 3079728 +slab_unreclaimable 381048 +slab 3460776 +workingset_refault_anon 0 +workingset_refault_file 30 +workingset_activate_anon 0 +workingset_activate_file 0 +workingset_restore_anon 0 +workingset_restore_file 0 +workingset_nodereclaim 0 +pgdemote_kswapd 0 +pgdemote_direct 0 +pgdemote_khugepaged 0 +pgscan 0 +pgsteal 0 +pgscan_kswapd 0 +pgscan_direct 0 +pgscan_khugepaged 0 +pgsteal_kswapd 0 +pgsteal_direct 0 +pgsteal_khugepaged 0 +pgfault 1807 +pgmajfault 2 +pgrefill 0 +pgactivate 0 +pgdeactivate 0 +pglazyfree 0 +pglazyfreed 0 +swpin_zero 0 +swpout_zero 0 +zswpin 0 +zswpout 0 +zswpwb 0 +thp_fault_alloc 0 +thp_collapse_alloc 0 +thp_swpout 0 +thp_swpout_fallback 0 diff --git a/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.current b/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.current new file mode 100644 index 0000000000..e1ec59cbbb --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.current @@ -0,0 +1 @@ +236740608 diff --git a/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.max b/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.max new file mode 100644 index 0000000000..853f47e70f --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.max @@ -0,0 +1 @@ +268435456 diff --git a/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.stat b/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.stat new file mode 100644 index 0000000000..5bd87d1422 --- /dev/null +++ b/agent-computer/tests/fixtures/cgroup/v2-shm-near-limit/memory.stat @@ -0,0 +1,61 @@ +anon 188416 +file 234881024 +kernel 1392640 +kernel_stack 32768 +pagetables 81920 +sec_pagetables 0 +percpu 44560 +sock 0 +vmalloc 8192 +shmem 234881024 +zswap 0 +zswapped 0 +file_mapped 0 +file_dirty 0 +file_writeback 0 +swapcached 0 +anon_thp 0 +file_thp 0 +shmem_thp 0 +inactive_anon 0 +active_anon 235053056 +inactive_file 0 +active_file 0 +unevictable 0 +slab_reclaimable 613960 +slab_unreclaimable 378376 +slab 992336 +workingset_refault_anon 0 +workingset_refault_file 0 +workingset_activate_anon 0 +workingset_activate_file 0 +workingset_restore_anon 0 +workingset_restore_file 0 +workingset_nodereclaim 0 +pgdemote_kswapd 0 +pgdemote_direct 0 +pgdemote_khugepaged 0 +pgscan 0 +pgsteal 0 +pgscan_kswapd 0 +pgscan_direct 0 +pgscan_khugepaged 0 +pgsteal_kswapd 0 +pgsteal_direct 0 +pgsteal_khugepaged 0 +pgfault 1780 +pgmajfault 0 +pgrefill 0 +pgactivate 0 +pgdeactivate 0 +pglazyfree 0 +pglazyfreed 0 +swpin_zero 0 +swpout_zero 0 +zswpin 0 +zswpout 0 +zswpwb 0 +thp_fault_alloc 0 +thp_collapse_alloc 0 +thp_swpout 0 +thp_swpout_fallback 0 diff --git a/agent-computer/tests/sessions.test.ts b/agent-computer/tests/sessions.test.ts index 86fc152f44..10b0e848bb 100644 --- a/agent-computer/tests/sessions.test.ts +++ b/agent-computer/tests/sessions.test.ts @@ -167,3 +167,21 @@ test("a replacement context interrupts a live handoff; the same context keeps it expect(session.control.get(request.id).request?.status).toBe("interrupted"); expect(session.control.humanMayDrive()).toBe(false); }); + +test("a Bot whose wheel a person holds, or has been asked to take, says so without a session being made", async () => { + // The computer asks this before closing a browser for memory or a Bot's idle tabs, and neither + // may close one under a person working in it, or under a page the Bot is waiting for them to work. + const sessions = createSessions({ isLive: () => true, mintRun: counting() }); + expect(sessions.personHasTheWheel("bot-1")).toBe(false); + // Asking did not make one: the paths that ask are the ones that close things. + expect(sessions.get("bot-1")).toBeUndefined(); + + const session = sessions.for("bot-1"); + expect(sessions.personHasTheWheel("bot-1")).toBe(false); + const request = session.control.requestHelp("Sign in").request!; + expect(sessions.personHasTheWheel("bot-1")).toBe(true); + await session.control.take(request.id); + expect(sessions.personHasTheWheel("bot-1")).toBe(true); + session.control.release(request.id); + expect(sessions.personHasTheWheel("bot-1")).toBe(false); +}); diff --git a/charts/openbot/templates/_helpers.tpl b/charts/openbot/templates/_helpers.tpl index 11ba5371ce..96ae909759 100644 --- a/charts/openbot/templates/_helpers.tpl +++ b/charts/openbot/templates/_helpers.tpl @@ -827,3 +827,16 @@ than anything that names the cause. {{- end }} {{- end -}} +{{- /* + A number from values written out in digits, which is how the computer reads its settings. + + A whole number in a values file arrives as a float, because Helm reads values through JSON, and a + float renders in Go's shortest form: `tabIdleMs: 1200000` became `1.2e+06`. The computer takes + digits only, so it fell back to its default without a word. A float with nothing after the point is + written as the whole number it is; anything else (a string, a number from `--set`) as it came. +*/}} +{{- define "openbot.wholeNumber" -}} +{{- if kindIs "float64" . -}} +{{- if eq . (floor .) -}}{{ int64 . }}{{- else -}}{{ . }}{{- end -}} +{{- else -}}{{ . }}{{- end -}} +{{- end -}} diff --git a/charts/openbot/templates/computer/statefulset.yaml b/charts/openbot/templates/computer/statefulset.yaml index fde0a21d84..e660547c14 100644 --- a/charts/openbot/templates/computer/statefulset.yaml +++ b/charts/openbot/templates/computer/statefulset.yaml @@ -163,6 +163,19 @@ spec: - name: COMPUTER_BROWSER_IDLE_MS value: {{ . | quote }} {{- end }} + {{- /* + Asked whether anybody set them, not whether they are truthy: zero switches each off, + and `with` reads zero as empty and would drop it, leaving the default on for a + deployment that had said off. Absent, as it is on a release from before these keys, + is nothing set. + */}} + {{- range $name, $key := dict "COMPUTER_MEMORY_HIGH_PERCENT" "memoryHighPercent" "COMPUTER_TAB_IDLE_MS" "tabIdleMs" }} + {{- $value := index $.Values.computers $key }} + {{- if and (not (kindIs "invalid" $value)) (ne (toString $value) "") }} + - name: {{ $name }} + value: {{ include "openbot.wholeNumber" $value | quote }} + {{- end }} + {{- end }} {{- with include "openbot.computerEnv" . | fromJsonArray }} {{ toYaml . | indent 12 }} {{- end }} diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index 21b6d281f3..a9ac345cd7 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -404,6 +404,15 @@ computers: # the process default, which is a handful and thirty minutes. maxBrowsers: "" browserIdleMs: "" + # At what share of the pod's memory limit the computer starts closing browsers, least recently used + # first, so a few heavy pages close one Bot's browser rather than the OOM killer taking the pod and + # every browser on it. Measured as the kubelet measures it (the working set). Empty means 85; 0 + # switches it off. + memoryHighPercent: "" + # How long a Bot's background tab (one it left behind, such as the page a sign-in popup opened from) + # is kept untouched before it is closed. The page the Bot is on is never closed. Empty means ten + # minutes; 0 keeps them. + tabIdleMs: "" # Which Chromium a Bot's computer runs. `headed` is full Chromium drawing to the pod's own X server # (Xvfb, started by the computer), behind the same streamed screen: it is what sites see from a diff --git a/docker-compose.yml b/docker-compose.yml index 306dde456d..c7a8ec8da9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -270,6 +270,11 @@ services: # minutes; closing a browser costs only a relaunch, because the profile is on the volume below. COMPUTER_MAX_BROWSERS: ${COMPUTER_MAX_BROWSERS:-} COMPUTER_BROWSER_IDLE_MS: ${COMPUTER_BROWSER_IDLE_MS:-} + # At what share of the container's memory it starts closing browsers, least recently used first + # (default 85), and how long a Bot's background tab may sit untouched (default ten minutes). A + # container with no --memory is measured against the machine's memory. 0 switches either off. + COMPUTER_MEMORY_HIGH_PERCENT: ${COMPUTER_MEMORY_HIGH_PERCENT:-} + COMPUTER_TAB_IDLE_MS: ${COMPUTER_TAB_IDLE_MS:-} # Every Bot without a computer of its own reaches this one, so each gets its own folder of # files and its shell starts there. The supervisor's per-Bot computers do not need it. COMPUTER_SEPARATE_WORKSPACES: "on" diff --git a/docker/s6/s6-rc.d/computer/run b/docker/s6/s6-rc.d/computer/run index 972d55e788..7d7614597b 100755 --- a/docker/s6/s6-rc.d/computer/run +++ b/docker/s6/s6-rc.d/computer/run @@ -26,7 +26,7 @@ for name in \ BUN_INSTALL PLAYWRIGHT_BROWSERS_PATH SPIFFE_ENDPOINT_SOCKET \ COMPUTER_TOKEN COMPUTER_BOT_ID COMPUTER_BROWSER_MODE COMPUTER_MAX_BROWSERS \ COMPUTER_BROWSER_IDLE_MS COMPUTER_SANDBOX COMPUTER_SHELL_ENV COMPUTER_STOPPED \ - COMPUTER_SEPARATE_WORKSPACES \ + COMPUTER_SEPARATE_WORKSPACES COMPUTER_MEMORY_HIGH_PERCENT COMPUTER_TAB_IDLE_MS \ ACTION_TIMEOUT_MS NAVIGATION_TIMEOUT_MS AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS do eval "isset=\${$name+x}" diff --git a/docs/configuration.md b/docs/configuration.md index 3cda4bb2e5..b42942ce2c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -651,6 +651,8 @@ is still the record. | `COMPUTER_MAX_BROWSERS` | How many Bots may hold a running browser at once. `8` by default; the least recently used is closed past it. A person's browser in a native window (OpenBot Desktop) counts toward it and is closed only when no Bot's browser is left to close; a person's cloud browser is ranked by use like a Bot's. | | `COMPUTER_SEPARATE_WORKSPACES` | `on` gives each Bot its own folder of files on a computer many Bots share, with its shell starting there. On for Helm's `shared` computer and the docker-compose `agent-computer`; off elsewhere. Files saved before it was on stay at the root of the volume. Commands can still read other folders: use `computers.mode: sandbox` to keep people apart. | | `COMPUTER_BROWSER_IDLE_MS` | How long an untouched browser is kept. 30 minutes by default; `0` keeps them resident. A person's cloud browser takes the same rule: the Browser page touches it every second while it is open, so it closes once nobody is on that page. A person's native window (OpenBot Desktop) is never closed for being idle. | +| `COMPUTER_MEMORY_HIGH_PERCENT` | At what share of its memory limit a computer closes browsers, least recently used first and one at a time, until it is back under. `85` by default; `0` switches it off. Memory is the container's working set, as the kubelet reads it (usage less the inactive page cache), checked before every launch and every minute. A container with no limit is measured against the machine's memory; a computer outside a container (OpenBot Desktop's) closes nothing for memory. Never closed for memory: the browser being launched, the most recently used one, and a Bot's whose wheel a person holds or has been asked to take. A person's native window goes last, as at the cap. Each close logs `computer-browser-closed-for-memory` with the working set and the limit in bytes. | +| `COMPUTER_TAB_IDLE_MS` | How long a Bot's background tab may sit untouched before it is closed. 10 minutes by default; `0` keeps them. A Bot is always on its newest page, so an older tab (the page a sign-in popup opened from, a link opened in a new tab) is one it cannot go back to. Navigating, being acted on, or being in front counts as use. Never closed: the page the Bot is on, a person's own browser, and any tab of a Bot whose wheel a person holds. | | `OPENBOT_RECORDER_DIR` | The built OpenBot Recorder (`extension/dist` after `bun run build`), absolute. Every person's browser the computer opens has it loaded and connected; without it, or pointed at anything that is not a built Recorder, the computer still serves Bots and refuses to open a person's browser, saying why. The computer image sets it to the Recorder it carries. | | `COMPUTER_PEOPLE_ONLY` | `on` for a computer that runs people's own browsers and no Bot (OpenBot Desktop sets it on its host-side computer); it refuses every Bot. A person's browser runs only on a computer of its own: this one, or one started for it alone (`COMPUTER_BOT_ID`, as the supervisor and a Sandbox start each computer). The shared computer refuses it, and so does the server, whose Browser page says why, because a Bot's shell there could read the person's logins off the disk. | | `COMPUTER_BROWSER_BACKEND` | `managed` by default (full bundled Chromium); `local-chrome` opts into installed Chrome with dedicated profiles and a loopback API. | diff --git a/scripts/check-new-values-keys.ts b/scripts/check-new-values-keys.ts index d80f7624a6..4dfa0e7c93 100644 --- a/scripts/check-new-values-keys.ts +++ b/scripts/check-new-values-keys.ts @@ -15,6 +15,9 @@ * * bun scripts/check-new-values-keys.ts [--since v0.0.4] */ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { parse, parseAllDocuments } from "yaml"; const [valuesFile, ...rest] = process.argv.slice(2); @@ -461,4 +464,73 @@ for (const { path, variable } of offSwitches) { } console.log(`${path}=0 stays zero`); } + +/* + * The computer's own settings, which the chart passes through only when somebody set them, so a + * blank leaves the computer's own default in force. Two things have gone wrong with that shape. + * + * ZERO. Each of these switches something off at zero, and `with` reads zero as empty, so a + * deployment that wrote `0` got nothing rendered and the default left on. + * + * A WHOLE NUMBER FROM A VALUES FILE. Helm reads values files through JSON, so `tabIdleMs: 1200000` + * arrives as a float and renders as `1.2e+06`. The computer reads digits only and falls back to its + * default without a word. `--set` parses an integer, which is why this needs a file of its own. + * + * Only where the chart runs the shared computer, which is the workload that carries them. + */ +const computerSettings: Array<{ + path: string; + variable: string; + whole: number; +}> = [ + { + path: "computers.memoryHighPercent", + variable: "COMPUTER_MEMORY_HIGH_PERCENT", + whole: 90, + }, + { + path: "computers.tabIdleMs", + variable: "COMPUTER_TAB_IDLE_MS", + whole: 1_200_000, + }, +]; +if (!baseline.out.includes("name: COMPUTER_SEPARATE_WORKSPACES")) { + console.log( + `${valuesFile} runs no shared computer, so its settings are not checked here`, + ); +} else { + const scratch = await mkdtemp(join(tmpdir(), "openbot-values-")); + try { + for (const { path, variable, whole } of computerSettings) { + const zero = render(["--set", `${path}=0`]); + const zeroGot = zero.ok ? renderedValue(zero.out, variable) : undefined; + if (zeroGot !== "0") { + console.error( + `::error::Setting ${path}=0 rendered ${variable}=${zeroGot ?? "nothing"} rather than 0. A zero is an off switch, not an absent value.`, + ); + bad += 1; + } else { + console.log(`${path}=0 stays zero`); + } + + const leaf = path.slice(path.lastIndexOf(".") + 1); + const file = join(scratch, `${leaf}.yaml`); + await Bun.write(file, `computers:\n ${leaf}: ${whole}\n`); + const fromFile = render(["--values", file]); + const fileGot = fromFile.ok + ? renderedValue(fromFile.out, variable) + : undefined; + if (fileGot !== String(whole)) { + console.error( + `::error::${leaf}: ${whole} in a values file rendered ${variable}=${fileGot ?? "nothing"}, which the computer does not read as ${whole}.`, + ); + bad += 1; + } else { + console.log(`${path} from a values file renders as ${fileGot}`); + } + } + } finally { + await rm(scratch, { recursive: true, force: true }); + } +} process.exit(bad === 0 ? 0 : 1); From eab041c2748fca60d45163bbd55d8a762a837282 Mon Sep 17 00:00:00 2001 From: David McKay Date: Wed, 28 Oct 2026 18:18:31 -0700 Subject: [PATCH 2/3] Send a computer's browser settings through Helm as written, to both kinds of computer computers.browserIdleMs and computers.maxBrowsers were rendered with `with`, so `browserIdleMs: 0` (keep browsers open) was dropped, and a whole number from a values file arrived as a float: an hour rendered as `3.6e+06`, which the computer refused for its default. All four browser settings now go through the shared computer environment, written in digits and kept at zero, so the computers `computers.mode: sandbox` makes get them too. The values check covers browserIdleMs alongside the two new keys. --- CHANGELOG.md | 6 ++++++ charts/openbot/templates/_helpers.tpl | 16 +++++++++++++- .../templates/computer/statefulset.yaml | 21 ------------------- scripts/check-new-values-keys.ts | 6 ++++++ 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4528022303..e7a1cfec1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,12 @@ nothing has used, navigated or brought to the front for ten minutes is now close is on is never closed, and neither is any tab of a person's own browser or of a Bot whose wheel a person holds. `COMPUTER_TAB_IDLE_MS` (Helm: `computers.tabIdleMs`) sets the time, and `0` keeps them. +Through Helm, `computers.browserIdleMs` and `computers.maxBrowsers` now reach the computer as +written. An hour (`browserIdleMs: 3600000`) used to render as `3.6e+06`, which the computer ignored +for its default of thirty minutes, and `browserIdleMs: 0` (keep browsers open) was dropped. All four +of these settings now also reach the computers `computers.mode: sandbox` makes, which had none of +them. + ### Generated interfaces use A2UI 1.0 With generative UI on, a Bot builds an interface from the A2UI 1.0 basic components instead of the diff --git a/charts/openbot/templates/_helpers.tpl b/charts/openbot/templates/_helpers.tpl index 96ae909759..9bad6a0a3e 100644 --- a/charts/openbot/templates/_helpers.tpl +++ b/charts/openbot/templates/_helpers.tpl @@ -679,7 +679,8 @@ podAntiAffinity: The computer settings this chart has values for, as a JSON list of env entries. One definition, read by the shared StatefulSet and by the per-Bot sandbox template, so a computer -cannot be headed in one mode and headless in the other, or proxied in one and not the other. +cannot be headed in one mode and headless in the other, proxied in one and not the other, or keep a +different number of browsers in one than the other. Each key is reached through a guard, because `helm upgrade --reuse-values` hands an existing release none of the keys this release added (see scripts/check-new-values-keys.ts). Absent `browserMode` @@ -691,6 +692,19 @@ leaves the computer to its own default, which is headed on Linux, the same thing {{- with $computers.browserMode -}} {{- $env = append $env (dict "name" "COMPUTER_BROWSER_MODE" "value" (toString .)) -}} {{- end -}} +{{- /* + How many browsers a computer keeps, and when it closes them and their idle tabs. Asked whether + anybody set each, not whether it is truthy: zero switches the idle closes and the memory check + off, and `with` reads zero as empty and would drop it. Written out in digits + (`openbot.wholeNumber`), because a whole number from a values file arrives as a float and + rendered `browserIdleMs: 3600000` as `3.6e+06`, which the computer refused for its default. +*/ -}} +{{- range $name, $key := dict "COMPUTER_MAX_BROWSERS" "maxBrowsers" "COMPUTER_BROWSER_IDLE_MS" "browserIdleMs" "COMPUTER_MEMORY_HIGH_PERCENT" "memoryHighPercent" "COMPUTER_TAB_IDLE_MS" "tabIdleMs" -}} +{{- $value := index $computers $key -}} +{{- if and (not (kindIs "invalid" $value)) (ne (toString $value) "") -}} +{{- $env = append $env (dict "name" $name "value" (include "openbot.wholeNumber" $value)) -}} +{{- end -}} +{{- end -}} {{- $egressProxy := $computers.egressProxy | default dict -}} {{- if $egressProxy.enabled -}} {{- $env = append $env (dict "name" "EGRESS_PROXY_DEFAULT" "valueFrom" (dict "secretKeyRef" (dict diff --git a/charts/openbot/templates/computer/statefulset.yaml b/charts/openbot/templates/computer/statefulset.yaml index e660547c14..03d3db417b 100644 --- a/charts/openbot/templates/computer/statefulset.yaml +++ b/charts/openbot/templates/computer/statefulset.yaml @@ -155,27 +155,6 @@ spec: */}} - name: COMPUTER_SEPARATE_WORKSPACES value: "on" - {{- with .Values.computers.maxBrowsers }} - - name: COMPUTER_MAX_BROWSERS - value: {{ . | quote }} - {{- end }} - {{- with .Values.computers.browserIdleMs }} - - name: COMPUTER_BROWSER_IDLE_MS - value: {{ . | quote }} - {{- end }} - {{- /* - Asked whether anybody set them, not whether they are truthy: zero switches each off, - and `with` reads zero as empty and would drop it, leaving the default on for a - deployment that had said off. Absent, as it is on a release from before these keys, - is nothing set. - */}} - {{- range $name, $key := dict "COMPUTER_MEMORY_HIGH_PERCENT" "memoryHighPercent" "COMPUTER_TAB_IDLE_MS" "tabIdleMs" }} - {{- $value := index $.Values.computers $key }} - {{- if and (not (kindIs "invalid" $value)) (ne (toString $value) "") }} - - name: {{ $name }} - value: {{ include "openbot.wholeNumber" $value | quote }} - {{- end }} - {{- end }} {{- with include "openbot.computerEnv" . | fromJsonArray }} {{ toYaml . | indent 12 }} {{- end }} diff --git a/scripts/check-new-values-keys.ts b/scripts/check-new-values-keys.ts index 4dfa0e7c93..51f60d5164 100644 --- a/scripts/check-new-values-keys.ts +++ b/scripts/check-new-values-keys.ts @@ -493,6 +493,12 @@ const computerSettings: Array<{ variable: "COMPUTER_TAB_IDLE_MS", whole: 1_200_000, }, + // Older than this check: an hour rendered as `3.6e+06`, and `0` ("keep browsers resident") as nothing. + { + path: "computers.browserIdleMs", + variable: "COMPUTER_BROWSER_IDLE_MS", + whole: 3_600_000, + }, ]; if (!baseline.out.includes("name: COMPUTER_SEPARATE_WORKSPACES")) { console.log( From bec3d4e168e71d006349c56efb0dee97e593337b Mon Sep 17 00:00:00 2001 From: David McKay Date: Thu, 29 Oct 2026 08:49:51 -0700 Subject: [PATCH 3/3] Close only browsers nobody is using for memory, one pass at a time, down to ten points under Review found four ways closing browsers for memory could hurt the work it was protecting. A launch counted only itself as in use, so a Bot that asked for its page a second ago and was waiting on its model could be closed in the middle of a run. The most recently used browser already running is now spared as well, and so is any browser used in the last minute (RECENTLY_USED_MS), which also keeps a launch from closing the browser the launch before it just started. Closing stopped at the line it started at, so the next launch crossed it again; a pass now starts at COMPUTER_MEMORY_HIGH_PERCENT and closes on to ten points under it. Every launch ran a pass of its own and read memory before any close had released it, so a burst of launches closed one browser each; one pass runs at a time and every caller waits on the one in flight. A person's own browser, cloud or native, is never closed for memory, because closing it ends their Recorder session without a word. The docs now call this a check before each launch and once a minute rather than a guarantee, and say that in the all-in-one image the share counts the database and the API server too, on purpose, because the kernel's out-of-memory kill counts them as well. createProfiles takes a clock, so the browser tests can make a browser a minute quiet without waiting a minute. --- .env.example | 10 +- CHANGELOG.md | 36 ++- agent-computer/src/browser-eviction.ts | 58 +++- agent-computer/src/container-memory.ts | 30 +- agent-computer/src/index.ts | 6 +- agent-computer/src/profiles.ts | 93 ++++--- agent-computer/tests/browser-eviction.test.ts | 109 +++++--- agent-computer/tests/browser-memory.test.ts | 259 ++++++++++++++---- agent-computer/tests/container-memory.test.ts | 30 +- charts/openbot/values.yaml | 9 +- docker-compose.yml | 7 +- docker/s6/s6-rc.d/computer/run | 6 + docs/configuration.md | 2 +- 13 files changed, 487 insertions(+), 168 deletions(-) diff --git a/.env.example b/.env.example index fb4bb07c8d..92595b42ec 100644 --- a/.env.example +++ b/.env.example @@ -361,10 +361,12 @@ COMPUTER_TOKEN= # When a computer closes a browser besides the cap (COMPUTER_MAX_BROWSERS) and the idle timeout # (COMPUTER_BROWSER_IDLE_MS). At or above this share of its memory limit, read as the kubelet reads it -# (the working set, so the page cache does not count), it closes browsers least recently used first -# until it is back under, so a few heavy pages cost one Bot a relaunch rather than the OOM killer -# taking every browser at once. A container with no memory limit is measured against the machine's -# memory; a computer outside a container closes nothing for memory. 0 switches it off. +# (the working set, so the page cache does not count), it closes browsers nobody has used for a +# minute, least recently used first, until it is ten points under. Memory is read before each launch +# and once a minute, so a page that grows between two checks can still run it out of memory. A +# container with no memory limit is measured against the machine's memory; a computer outside a +# container closes nothing for memory. In the all-in-one image the share counts the database and the +# API server too, on purpose: the out-of-memory kill counts them as well. 0 switches it off. # COMPUTER_MEMORY_HIGH_PERCENT=85 # How long a Bot's background tab may sit untouched before it is closed, in ms: a tab the Bot left # behind, such as the page a sign-in popup opened from. The page the Bot is on, a person's own diff --git a/CHANGELOG.md b/CHANGELOG.md index e7a1cfec1c..96333b0ad1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,12 +61,13 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. to keep the old behaviour. - Interfaces a Bot drew before the upgrade (A2UI 0.9 surfaces) no longer draw when a conversation is reopened; the messages around them stay. New ones use A2UI 1.0 (below). -- A computer now closes browsers, least recently used first, when its memory reaches 85% of its - limit, and closes a Bot's background tabs after ten minutes untouched (below). +- A computer now closes browsers nobody is using, least recently used first, when its memory + reaches 85% of its limit, and closes a Bot's background tabs after ten minutes untouched (below). + In the all-in-one image that share counts the database and the API server too. `COMPUTER_MEMORY_HIGH_PERCENT=0` and `COMPUTER_TAB_IDLE_MS=0` (Helm: `computers.memoryHighPercent` and `computers.tabIdleMs`) keep the old behaviour. -### A computer closes a browser before its memory runs out +### A computer closes browsers nobody is using when its memory runs high A computer runs one browser for each Bot that is browsing, and the only limits were a count and half an hour untouched. Neither looked at memory, so a few heavy pages could take a container past its @@ -75,14 +76,27 @@ was busy. On Kubernetes it kills everything in the container at once, so every B together and nothing could reach the computer until it came back. A computer now reads its own memory the way the kubelet does (the working set, which leaves out the file cache the kernel can drop), before it starts a browser and once a minute. At or above 85% of its limit it closes browsers -one at a time, least recently used first, until it is back under. A Bot whose browser was closed -starts again where it left off on its next step, still signed in, because its profile is on disk. -The browser being started, the one used most recently, and the browser of a Bot whose wheel a person -holds or has been asked to take are never closed for memory. A container with no memory limit is -measured against the machine's memory, and a computer outside a container, such as OpenBot -Desktop's, closes nothing for memory. Each close is logged as `computer-browser-closed-for-memory` -with the working set and the limit in bytes. `COMPUTER_MEMORY_HIGH_PERCENT` (Helm: -`computers.memoryHighPercent`) sets the share, and `0` switches it off. +one at a time, least recently used first, until it is under 75%, so the next launch does not cross +the line again straight away. Launches that arrive together share one pass rather than closing a +browser each. A Bot whose browser was closed starts again where it left off on its next step, still +signed in, because its profile is on disk. + +Only a browser nobody is using is closed. Never closed for memory: the browser being started, the +most recently used one, any browser used in the last minute (a Bot between two steps asks for +nothing while its model works out the next), a person's own browser, cloud or native, whose Recorder +session would end, and the browser of a Bot whose wheel a person holds or has been asked to take. +When every browser is in use, none is closed. + +This is a check, not a guarantee. Memory is read before each launch and once a minute, so a page +that grows between two checks can still run the container out of memory. A container with no memory +limit is measured against the machine's memory, and a computer outside a container, such as OpenBot +Desktop's, closes nothing for memory. In the all-in-one image the database and the API server run in +the same container as the browsers, and the share counts them too. That is deliberate: the kernel's +out-of-memory kill counts everything in the container and can take the database down, and closing a +browser nobody is using first is how the database is protected. Each close is logged as +`computer-browser-closed-for-memory` with the working set and the limit in bytes. +`COMPUTER_MEMORY_HIGH_PERCENT` (Helm: `computers.memoryHighPercent`) sets the share where closing +starts, and `0` switches it off. The tabs a Bot leaves behind are closed too. A Bot always works in its newest page, so an older tab, such as the page a sign-in popup was opened from or a link a site opened in a new tab, is one it diff --git a/agent-computer/src/browser-eviction.ts b/agent-computer/src/browser-eviction.ts index e0ae9989dd..bdcbb3015d 100644 --- a/agent-computer/src/browser-eviction.ts +++ b/agent-computer/src/browser-eviction.ts @@ -96,32 +96,62 @@ function inClosingOrder( ); } +/** + * How long after its last use a browser still counts as in use, and so is never closed for memory. + * + * A browser's use is recorded when something asks for its page: a Bot's step, a person's input, the + * live screen. Between two steps a Bot asks for nothing while its model works out the next one, which + * can take tens of seconds, and nothing here can tell that pause from a Bot that has finished. + * Closing the browser then closes it in the middle of a run: the page the Bot was on is gone, and its + * next step starts a cold browser, which is the very jump in memory the close was meant to head off. + * A minute covers a model's turn between steps. It also covers a browser that has just launched, so + * the next launch never closes the one before it, which is what would otherwise turn a full computer + * into a loop of each Bot's launch closing the last Bot's browser. + */ +export const RECENTLY_USED_MS = 60_000; + /** * Which to close next because the computer's memory runs high (`container-memory.ts`). * * One at a time, because the caller reads memory again after each close and stops as soon as it is - * enough: a count says in advance how many have to go, and bytes do not. + * enough: a count says in advance how many have to go, and bytes do not. Least recently used first, + * the cap's order, so the two never disagree about who goes first. * - * The cap's order, so the two never disagree about who goes first: least recently used, with a - * person's native window last. And the cap's protection: the newest is never chosen. Before a - * launch the caller counts the Bot that is asking as the newest, so the browser being launched is - * never what makes room for itself; on the idle sweep the newest is whoever was used last. Either - * way at least one browser is always left, because one browser that is on its own too big for the - * computer is the limit's problem, and closing it would only start it again on the next request. + * Never a browser in use, which here means four things: + * - The one being launched. The caller runs this before a launch and passes only the browsers + * already running, so the Bot that asked is not among them. + * - The most recently used of those, however long ago that was. So at least one browser is always + * left, because one browser that is on its own too big for the computer is the limit's problem, + * and closing it would only start it again on its next step. + * - Any used within `recentlyUsedMs` ({@link RECENTLY_USED_MS}), which is a Bot in the middle of a + * run or a browser that has just started. + * - Any `spared` names, which is a Bot whose wheel a person holds or has been asked to take: closing + * it closes it under somebody's hands, or under a page the Bot is waiting for somebody to work. * - * Never one `spared` names, which is a Bot whose wheel a person holds or has been asked to take. - * Closing that browser closes it under somebody's hands, or under a page the Bot is waiting for - * somebody to come and work, and memory pressure is not a reason to do either. + * And never a person's own browser, cloud or native. Their Recorder runs in it, and closing it ends + * what they were recording without a word to them. It still counts toward the cap, which closes it + * last; memory pressure is not a reason to end somebody's session. */ export function chooseForMemory( running: Iterable<[string, Evictable]>, - goesLast: (botId: string) => boolean = () => false, - spared: (botId: string) => boolean = () => false, + now: number, + { + recentlyUsedMs = RECENTLY_USED_MS, + spared = () => false, + }: { + recentlyUsedMs?: number; + spared?: (botId: string) => boolean; + } = {}, ): string | undefined { const entries = [...running]; - return inClosingOrder(entries, newestOf(entries), goesLast) + const cutoff = now - recentlyUsedMs; + return inClosingOrder(entries, newestOf(entries), () => false) + .filter( + ([botId, entry]) => + entry.usedAt <= cutoff && !isPersonBrowserId(botId) && !spared(botId), + ) .map(([botId]) => botId) - .find((botId) => !spared(botId)); + .at(0); } /** diff --git a/agent-computer/src/container-memory.ts b/agent-computer/src/container-memory.ts index fc2ee3c88b..613f3d416b 100644 --- a/agent-computer/src/container-memory.ts +++ b/agent-computer/src/container-memory.ts @@ -164,6 +164,14 @@ export async function readContainerMemory({ return cgroupV1Memory({ usage, limit, stat: v1Stat }, machineBytes); } +/** Whether the working set is at or above a share of the limit. */ +export function memoryAtOrAbove( + reading: MemoryReading, + percent: number, +): boolean { + return reading.workingSet * 100 >= reading.limit * percent; +} + /** * Whether the computer is close enough to its limit to start closing browsers. * @@ -176,5 +184,25 @@ export function memoryRunsHigh( highPercent: number, ): boolean { if (highPercent <= 0) return false; - return reading.workingSet * 100 >= reading.limit * highPercent; + return memoryAtOrAbove(reading, highPercent); +} + +/** + * How far under the share that starts the closing memory has to fall before the closing stops. + * + * Stopping at the same line it started at leaves the computer a browser's launch from crossing it + * again, so the next Bot to start closes another browser, whose Bot starts again and closes another: + * each launch paying for the last. Closing on to ten points under gives the launches after it room. + * Ten points, not a setting of its own: it is a gap between two numbers, and the one an operator + * chooses is where the closing starts. + */ +export const MEMORY_TARGET_POINTS_UNDER = 10; + +/** + * The share closing stops under: {@link MEMORY_TARGET_POINTS_UNDER} below where it starts, so 75 at + * the default 85. At or under zero, which only a starting share of ten or less gives, closing goes on + * while anything is left that may be closed. + */ +export function memoryTargetPercent(highPercent: number): number { + return highPercent - MEMORY_TARGET_POINTS_UNDER; } diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index c741ff631a..d79d3b9b14 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -312,9 +312,9 @@ const profiles = createProfiles( }, { personDisplay: PERSON_DISPLAY, - // A browser a person is working in, or has been asked to, is never closed for memory, and none - // of its tabs are closed for being idle. A person's own browser is not a Bot's wheel at all: it - // takes the cap's rules, a native window last and a cloud one by use. + // A Bot's browser a person is working in, or has been asked to, is never closed for memory, and + // none of its tabs are closed for being idle. A person's own browser has no Bot's wheel to hold: + // it is never closed for memory whoever is at it (`chooseForMemory`), and its tabs are never swept. heldByPerson: (botId) => !isPersonBrowserId(botId) && sessions.personHasTheWheel(botId), }, diff --git a/agent-computer/src/profiles.ts b/agent-computer/src/profiles.ts index 816cfd47aa..d22014396b 100644 --- a/agent-computer/src/profiles.ts +++ b/agent-computer/src/profiles.ts @@ -43,13 +43,14 @@ import { chooseForMemory, chooseIdle, chooseIdleTabs, - type Evictable, keptOpen, } from "./browser-eviction"; import { type BrowserRuntime, browserRuntimeFromEnv } from "./browser-runtime"; import { type MemoryReading, + memoryAtOrAbove, memoryRunsHigh, + memoryTargetPercent, readContainerMemory, } from "./container-memory"; import { egressFor, egressLabel } from "./egress"; @@ -326,17 +327,23 @@ const IDLE_SWEEP_MS = 60_000; * At what share of the computer's memory it starts closing browsers, least recently used first. * * The cap counts browsers, and a browser is not a fixed size: a few heavy pages can take a container - * past its memory limit well under the cap, and the OOM killer does not choose by use: on Kubernetes + * past its memory limit well under the cap, and the OOM killer does not choose by use. On Kubernetes * with cgroup v2 it kills every process in the container together, every Bot's browser at once, and - * nothing can reach the computer until it comes back. So memory is read the way Kubernetes reads it (`container-memory.ts`) - * before every launch, which is when memory jumps, and on every tick of the idle sweep, and while it - * is at or above this share of the limit the least recently used browser is closed and memory read - * again. Closing one costs its Bot a relaunch and nothing else, the same as the cap. + * nothing can reach the computer until it comes back. So memory is read the way Kubernetes reads it + * (`container-memory.ts`) before every launch, which is when memory jumps, and on every tick of the + * idle sweep. At or above this share of the limit, browsers are closed one at a time, least recently + * used first, until memory is ten points under it (`memoryTargetPercent`). Closing one costs its Bot + * a relaunch and nothing else, the same as the cap. + * + * A check, not a guarantee. Between two checks a page can grow on its own, and memory can run out + * before the next launch or the next minute comes round. What this removes is the computer filling + * itself up one launch at a time with browsers nobody is using. * * Eighty-five leaves room for the launch that asked, and for a page that grows after the check, * before the limit is reached. Zero switches it off, as it does for the idle sweep. A computer with no * cgroup of its own (OpenBot Desktop's, on a person's Mac) has no limit to read and closes nothing - * for memory either way. + * for memory either way. In the all-in-one image the share counts the database and the API server as + * well, deliberately: the OOM kill counts them too, and a browser closed first is what keeps them up. */ const MEMORY_HIGH_PERCENT = numberFromEnv("COMPUTER_MEMORY_HIGH_PERCENT", 85, { zeroSwitchesItOff: true, @@ -401,6 +408,11 @@ export type ProfilesOptions = { memoryHighPercent?: number; /** {@link TAB_IDLE_MS}, for a test that cannot wait ten minutes. */ tabIdleMs?: number; + /** + * The clock every use, idle timeout and memory decision here is measured by. `Date.now` unless a + * test needs a browser to have gone quiet a minute ago without waiting a minute for it. + */ + now?: () => number; /** * Whether a person holds this Bot's wheel, or has been asked to take it. Such a browser is never * closed for memory, and none of its tabs are closed for being idle: a person may be working across @@ -439,6 +451,7 @@ export function createProfiles( const readMemory = options.memory ?? (() => readContainerMemory()); const memoryHighPercent = options.memoryHighPercent ?? MEMORY_HIGH_PERCENT; const tabIdleMs = options.tabIdleMs ?? TAB_IDLE_MS; + const now = options.now ?? Date.now; const heldByPerson = options.heldByPerson ?? (() => false); /* * A wheel that cannot be read counts as held. The control state throws when its store has failed, @@ -492,11 +505,11 @@ export function createProfiles( */ const tabUsedAt = new WeakMap(); const touchTab = (page: Page): void => { - tabUsedAt.set(page, Date.now()); + tabUsedAt.set(page, now()); }; /** A tab never seen before counts as used now, so one this process missed is not closed at once. */ const tabLastUsed = (page: Page): number => { - const at = tabUsedAt.get(page) ?? Date.now(); + const at = tabUsedAt.get(page) ?? now(); tabUsedAt.set(page, at); return at; }; @@ -577,25 +590,26 @@ export function createProfiles( }; /** - * Close browsers, least recently used first, while the computer's memory runs high. + * Close browsers, least recently used first, once the computer's memory runs high, until it is ten + * points under the line. * * One at a time, reading memory again after each close, because what one browser was holding is - * not known until it has gone. `asking` is the Bot whose launch this runs ahead of: counted as the - * newest, so it is never what makes room for itself, while every other browser here can be. Stops - * when memory is back under the line, when nothing is left that may be closed (`chooseForMemory`), - * or when the same browser comes up twice, which a close racing this one could otherwise loop on. + * not known until it has gone. Starts at {@link MEMORY_HIGH_PERCENT} and goes on to + * `memoryTargetPercent` under it, so the launch after it does not cross the line again at once. + * Stops when memory is there, when nothing is left that may be closed (`chooseForMemory`: never + * the browser launching, the most recently used, one in use within the last minute, one a person + * holds the wheel of, or a person's own), or when the same browser comes up twice. */ - const relieveMemory = async (asking?: string): Promise => { + const relieveMemoryOnce = async (): Promise => { if (memoryHighPercent <= 0) return; + // A reading that fails is a computer with nothing to act on, the same as one with no cgroup. + const read = () => readMemory().catch(() => undefined); + let reading = await read(); + if (!reading || !memoryRunsHigh(reading, memoryHighPercent)) return; + const target = memoryTargetPercent(memoryHighPercent); const tried = new Set(); - for (;;) { - // A reading that fails is a computer with nothing to act on, the same as one with no cgroup. - const reading = await readMemory().catch(() => undefined); - if (!reading || !memoryRunsHigh(reading, memoryHighPercent)) return; - const running: [string, Evictable][] = [...live.entries()]; - if (asking && !live.has(asking)) - running.push([asking, { usedAt: Date.now() }]); - const next = chooseForMemory(running, stays, spared); + while (reading && memoryAtOrAbove(reading, target)) { + const next = chooseForMemory(live.entries(), now(), { spared }); if (!next || tried.has(next)) return; tried.add(next); console.info( @@ -607,9 +621,26 @@ export function createProfiles( }), ); await evict(next, "the computer's memory ran high"); + reading = await read(); } }; + /** + * One pass at a time, which every caller waits on. + * + * A burst of launches each ran a pass of its own, and each read memory before any close had given + * any back: a browser's memory is released when its Chromium exits, seconds after it was taken out + * of the map. So K launches at once read the same high number and closed K browsers where one was + * enough. A launch that arrives while a pass is running waits for that pass and launches after it. + */ + let relieving: Promise | undefined; + const relieveMemory = (): Promise => { + relieving ??= relieveMemoryOnce().finally(() => { + relieving = undefined; + }); + return relieving; + }; + /** * Close the background tabs of every Bot's browser that nothing has touched for a while. * @@ -621,7 +652,7 @@ export function createProfiles( */ const closeIdleTabs = async (): Promise => { if (tabIdleMs <= 0) return 0; - const now = Date.now(); + const sweptAt = now(); let closed = 0; for (const [botId, running] of [...live.entries()]) { if (running.person || isPersonBrowserId(botId) || spared(botId)) continue; @@ -639,7 +670,7 @@ export function createProfiles( page.url().startsWith("chrome-extension://") || page.url().startsWith("devtools://"), })); - for (const { page } of chooseIdleTabs(tabs, tabIdleMs, now)) { + for (const { page } of chooseIdleTabs(tabs, tabIdleMs, sweptAt)) { console.info( JSON.stringify({ type: "computer-tab-closed", @@ -667,7 +698,7 @@ export function createProfiles( for (const botId of chooseIdle( live.entries(), IDLE_TIMEOUT_MS, - Date.now(), + now(), stays, )) { await evict(botId, "it had been idle"); @@ -821,7 +852,7 @@ export function createProfiles( context, page, startedAt: new Date().toISOString(), - usedAt: Date.now(), + usedAt: now(), retarget: () => {}, ...(sessionCookies ? { sessionCookies } : {}), person, @@ -918,7 +949,7 @@ export function createProfiles( ) { // Touched on every use, which is what makes "least recently used" mean anything. The page in // front with it: a Bot's next action, and a person's input on the live screen, land there. - existing.usedAt = Date.now(); + existing.usedAt = now(); touchTab(existing.page); return existing.page; } @@ -945,7 +976,7 @@ export function createProfiles( const launch = (async () => { // Before the launch, which is when memory jumps: a cold Chromium and its first page are the // largest single step up a computer takes, and the OOM killer does not wait for the cap. - await relieveMemory(botId); + await relieveMemory(); const dir = directoryFor(botId); // A second native helper can target the same data root on another port. Do not remove an // active Chrome profile's lock; Chrome reports contention and handles its own stale locks. @@ -998,7 +1029,7 @@ export function createProfiles( context, page, startedAt: new Date().toISOString(), - usedAt: Date.now(), + usedAt: now(), retarget: () => {}, ...(sessionCookies ? { sessionCookies } : {}), }; @@ -1164,7 +1195,7 @@ export function createProfiles( const pages = () => personPages(running.context, person.recorderId); if (!windowPages(running.context).includes(running.page)) return undefined; - running.usedAt = Date.now(); + running.usedAt = now(); return { ...person, context: running.context, diff --git a/agent-computer/tests/browser-eviction.test.ts b/agent-computer/tests/browser-eviction.test.ts index 3c5f9e3ae6..db569d129d 100644 --- a/agent-computer/tests/browser-eviction.test.ts +++ b/agent-computer/tests/browser-eviction.test.ts @@ -5,6 +5,7 @@ import { chooseIdle, chooseIdleTabs, keptOpen, + RECENTLY_USED_MS, } from "../src/browser-eviction"; import { numberFromEnv } from "../src/env"; @@ -264,59 +265,89 @@ describe("a person's cloud browser among Bots' browsers", () => { * * The cap counts browsers and the idle sweep counts minutes, and neither looks at memory, so a few * heavy pages took a container past its limit and the OOM killer took every browser on it at once. - * The computer now closes browsers one at a time while memory runs high, reading it again after each. + * The computer now closes browsers one at a time while memory runs high, reading it again after each, + * and never one that is in use: a Bot between two steps asks for nothing while its model thinks, and + * closing its browser then closes it in the middle of a run. */ describe("closing a browser because memory runs high", () => { + const NOW = 10_000_000; + const MINUTE = 60_000; const PERSON = "person-0123456789abcdef0123456789abcdef"; - const isPerson = (botId: string) => botId.startsWith("person-"); + /** Bots used this long before NOW, in order. */ + const usedAgo = (...ago: number[]) => running(...ago.map((ms) => NOW - ms)); - test("closes the least recently used, by the cap's own order", () => { - expect(chooseForMemory(running(500, 100, 300, 900))).toBe("bot-1"); - }); - - test("never the newest, which before a launch is the Bot asking", () => { - // The caller adds the Bot whose browser is about to launch as the most recently used, so it is - // never what makes room for itself, however everything else is ranked. - const before = running(100, 200); - before.set("asking", { usedAt: 300 }); - const chosen: string[] = []; - for (;;) { - const next = chooseForMemory(before); - if (!next) break; - chosen.push(next); - before.delete(next); - } - expect(chosen).toEqual(["bot-0", "bot-1"]); - expect([...before.keys()]).toEqual(["asking"]); + test("closes the least recently used of the browsers nobody has used for a minute", () => { + expect( + chooseForMemory(usedAgo(5 * MINUTE, 9 * MINUTE, 2 * MINUTE, 1_000), NOW), + ).toBe("bot-1"); }); - test("never the last browser left: one too big on its own is the limit's problem", () => { - expect(chooseForMemory(running(100))).toBeUndefined(); - expect(chooseForMemory(running())).toBeUndefined(); + test("never a Bot used seconds ago, which is a Bot whose model is working out its next step", () => { + // bot-0 asked for its page ten seconds ago and has asked for nothing since. It is the least + // recently used here and it is still in the middle of its run. + expect(chooseForMemory(usedAgo(10_000, 5_000), NOW)).toBeUndefined(); + expect(chooseForMemory(usedAgo(10_000, 3 * MINUTE, 5_000), NOW)).toBe( + "bot-1", + ); }); - test("never a browser whose wheel a person holds", () => { - // bot-0 is the least recently used and a person is working in it: the next one goes instead. - const held = (botId: string) => botId === "bot-0"; - expect(chooseForMemory(running(100, 200, 300), undefined, held)).toBe( - "bot-1", + test("a minute is the line: used exactly a minute ago may close, a moment inside it may not", () => { + expect(chooseForMemory(usedAgo(MINUTE, 0), NOW)).toBe("bot-0"); + expect(chooseForMemory(usedAgo(MINUTE - 1, 0), NOW)).toBeUndefined(); + expect(RECENTLY_USED_MS).toBe(MINUTE); + }); + + test("never the most recently used browser running, however long ago that was", () => { + // Before a launch the caller passes the browsers already running, and the one launching is not + // among them. Of those, the newest stays even when every one is an hour quiet, so one is left. + expect(chooseForMemory(usedAgo(60 * MINUTE), NOW)).toBeUndefined(); + expect(chooseForMemory(usedAgo(90 * MINUTE, 60 * MINUTE), NOW)).toBe( + "bot-0", ); - // With only the held one and the newest left, nothing is closed. - expect(chooseForMemory(running(100, 300), undefined, held)).toBeUndefined(); + expect(chooseForMemory(running(), NOW)).toBeUndefined(); + }); + + test("a browser that has just launched is never what the next launch closes", () => { + // The loop this rules out: each Bot's launch closing the browser the Bot before it just started. + // Here the one just launched is not the newest (another Bot asked a second ago), so only the + // minute keeps it. + const justLaunched = new Map([ + ["just-launched", { usedAt: NOW - 2_000 }], + ["asked-a-second-ago", { usedAt: NOW - 1_000 }], + ["quiet", { usedAt: NOW - 20 * MINUTE }], + ]); + expect(chooseForMemory(justLaunched, NOW)).toBe("quiet"); + justLaunched.delete("quiet"); + expect(chooseForMemory(justLaunched, NOW)).toBeUndefined(); }); - test("a person's native window goes after every Bot's browser, as at the cap", () => { + test("never a person's own browser, cloud or native, even the one used longest ago", () => { + // Their Recorder runs in it, and closing it ends what they were recording without a word. const withPerson = new Map([ - [PERSON, { usedAt: 1 }], - ...running(500, 300, 900), + [PERSON, { usedAt: NOW - 120 * MINUTE }], + ...usedAgo(30 * MINUTE, 10 * MINUTE), ]); - expect(chooseForMemory(withPerson, isPerson)).toBe("bot-1"); + expect(chooseForMemory(withPerson, NOW)).toBe("bot-0"); + withPerson.delete("bot-0"); + expect(chooseForMemory(withPerson, NOW)).toBeUndefined(); + }); + + test("never a browser whose wheel a person holds", () => { + const held = (botId: string) => botId === "bot-0"; expect( - chooseForMemory( - new Map([[PERSON, { usedAt: 1 }], ...running(900)]), - isPerson, - ), - ).toBe(PERSON); + chooseForMemory(usedAgo(30 * MINUTE, 20 * MINUTE, 10 * MINUTE), NOW, { + spared: held, + }), + ).toBe("bot-1"); + expect( + chooseForMemory(usedAgo(30 * MINUTE, 10 * MINUTE), NOW, { spared: held }), + ).toBeUndefined(); + }); + + test("the window can be narrowed, which is how the browser tests run without waiting a minute", () => { + expect(chooseForMemory(usedAgo(500, 0), NOW, { recentlyUsedMs: 100 })).toBe( + "bot-0", + ); }); }); diff --git a/agent-computer/tests/browser-memory.test.ts b/agent-computer/tests/browser-memory.test.ts index 50668eb623..d300790c8b 100644 --- a/agent-computer/tests/browser-memory.test.ts +++ b/agent-computer/tests/browser-memory.test.ts @@ -2,6 +2,7 @@ import { afterAll, afterEach, describe, expect, spyOn, test } from "bun:test"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import type { BrowserContext, Page } from "playwright"; import type { MemoryReading } from "../src/container-memory"; /** @@ -79,91 +80,200 @@ async function profilesFor( return { profiles, told }; } -/** - * A container where each running browser holds the same 450 MiB of a 1000 MiB limit, read from how - * many are running now, as the cgroup would read it. Two browsers are 90% (high at 85), one is 45%. - */ const MIB = 1024 ** 2; const LIMIT = 1000 * MIB; -const PER_BROWSER = 450 * MIB; -const perBrowser = - (running: () => number) => async (): Promise => ({ - workingSet: running() * PER_BROWSER, - limit: LIMIT, - }); +const MINUTE = 60_000; +const PERSON = "person-0123456789abcdef0123456789abcdef"; + +/** + * A container whose memory is its browsers, read the way the cgroup reads it. + * + * Each browser holds the same share, counted while its Chromium is still connected, because that is + * when the kernel gives its memory back: when the process exits, seconds after the computer took it + * out of its map, not at the moment it decided to close it. The browsers are real; only their size + * is set, so a test can say how many fit. + */ +function containerOf(percentEach: number) { + const contexts = new Set(); + return { + /** Count this page's browser from now on. */ + holds(page: Page): Page { + contexts.add(page.context()); + return page; + }, + memory: async (): Promise => ({ + workingSet: + [...contexts].filter((context) => context.browser()?.isConnected()) + .length * + percentEach * + 10 * + MIB, + limit: LIMIT, + }), + }; +} + +/** + * A clock that can be moved on, so every browser used so far reads as quiet for as long as a test + * says, without the test waiting a minute for the window in which a browser counts as in use. + */ +function clock() { + let ahead = 0; + return { + now: () => Date.now() + ahead, + passes(ms: number) { + ahead += ms; + }, + }; +} + +const closedForMemory = () => + logged("computer-browser-closed-for-memory").map((line) => line.botId); describe.skipIf(!asked)("closing browsers when memory runs high", () => { test( - "a launch on a computer whose memory runs high closes the least recently used first, never the one launching", + "a Bot used seconds ago is not closed when another launches; the quiet ones are", async () => { - let profiles: Awaited>["profiles"]; - const made = await profilesFor("memory", { - memory: perBrowser(() => profiles.liveCount()), - memoryHighPercent: 85, + // Two browsers are 90%, over the line at 85. + const container = containerOf(45); + const time = clock(); + const { profiles, told } = await profilesFor("memory-recent", { + memory: container.memory, + now: time.now, }); - profiles = made.profiles; + const open = async (botId: string) => + container.holds(await profiles.page(botId)); try { - // One browser is 45%: the second launches without anything being closed. - await profiles.page("first"); - await profiles.page("second"); - expect(made.told).toEqual([]); - // The first is used again, so the second is now the least recently used: by use, not by - // which started first. A moment later, because use is kept to the millisecond and the - // second's launch can finish inside the same one. - await new Promise((resolve) => setTimeout(resolve, 5)); - await profiles.page("first"); + await open("first"); + await open("second"); + // Memory is high, and both Bots asked for their pages seconds ago: either could be waiting on + // its model between two steps. Neither is closed, and the third launches anyway. + await open("third"); + expect(told).toEqual([]); + expect(profiles.liveCount()).toBe(3); - // Two browsers are 90%. The third Bot's launch closes the second, reads 45%, and launches. - const third = await profiles.page("third"); + // Ten minutes on, the first Bot takes a step. The other two have been quiet all that time. + time.passes(10 * MINUTE); + await open("first"); + await open("fourth"); - expect(made.told).toEqual(["second"]); + // Both quiet ones close, down to under 75: three browsers were 135%, one is 45%. + expect(told).toEqual(["second", "third"]); expect(profiles.isLive("first")).toBe(true); - expect(profiles.isLive("second")).toBe(false); - expect(profiles.isLive("third")).toBe(true); - expect(third.isClosed()).toBe(false); - expect(logged("computer-browser-closed-for-memory")).toEqual([ - { - type: "computer-browser-closed-for-memory", - botId: "second", - workingSetBytes: 2 * PER_BROWSER, - limitBytes: LIMIT, - }, - ]); + expect(profiles.isLive("fourth")).toBe(true); + expect(logged("computer-browser-closed-for-memory")[0]).toEqual({ + type: "computer-browser-closed-for-memory", + botId: "second", + workingSetBytes: 3 * 450 * MIB, + limitBytes: LIMIT, + }); } finally { - await made.profiles.closeAll(); + await profiles.closeAll(); + } + }, + LAUNCH_TIMEOUT_MS, + ); + + test( + "a burst of launches closes only what memory needs, not one browser per launch", + async () => { + // Three browsers are 90%; one closed is 60%, under 75. + const container = containerOf(30); + const time = clock(); + const { profiles, told } = await profilesFor("memory-burst", { + memory: container.memory, + now: time.now, + }); + const open = async (botId: string) => + container.holds(await profiles.page(botId)); + try { + await open("a"); + await open("b"); + await open("c"); + time.passes(10 * MINUTE); + + // Three Bots at once. Each launch used to read memory itself, before the first close had + // given anything back, and each closed a browser of its own. + await Promise.all([open("d"), open("e"), open("f")]); + + expect(told).toEqual(["a"]); + expect(closedForMemory()).toEqual(["a"]); + expect(profiles.liveCount()).toBe(5); + } finally { + await profiles.closeAll(); } }, LAUNCH_TIMEOUT_MS, ); test( - "a browser a person is working in stays, and memory that will not come down never closes the last browser", + "a Bot that comes back after its browser was closed does not start a cycle of closes", async () => { - // Memory that reads high whatever is closed: one browser too big on its own. The loop has to - // stop at the newest, not close its way to nothing. - const made = await profilesFor("memory-held", { + // Four browsers are 100%. One closed is 75%, which is under where closing starts (85) and not + // under where it stops (75), so the pass goes on to 50%. + const container = containerOf(25); + const time = clock(); + const { profiles, told } = await profilesFor("memory-cycle", { + memory: container.memory, + now: time.now, + }); + const open = async (botId: string) => + container.holds(await profiles.page(botId)); + try { + for (const botId of ["a", "b", "c", "d"]) await open(botId); + time.passes(10 * MINUTE); + + await open("e"); + // Two, not one: a pass that stopped at 75% left the next launch at 100% again. + expect(told).toEqual(["a", "b"]); + + // Each Bot whose browser was closed comes straight back, as a Bot in the middle of its work + // would. Each comeback used to close the least recently used browser, which by then was the + // one the last comeback had just launched, and so on round the ring for ever. + for (let round = 0; round < 8; round += 1) { + const closed = told.find((botId) => !profiles.isLive(botId)); + if (!closed) break; + await open(closed); + } + + // Every quiet browser was closed once, and none that was in use: the Bots are all back. + expect(told).toEqual(["a", "b", "c", "d"]); + expect(new Set(told).size).toBe(told.length); + for (const botId of ["a", "b", "c", "d", "e"]) + expect(profiles.isLive(botId)).toBe(true); + } finally { + await profiles.closeAll(); + } + }, + LAUNCH_TIMEOUT_MS, + ); + + test( + "a Bot a person is working in stays, and memory that will not come down leaves the newest", + async () => { + // Memory that reads full whatever is closed: one browser too big on its own. The pass has to + // stop, not close its way to nothing. + const time = clock(); + const { profiles, told } = await profilesFor("memory-held", { memory: async () => ({ workingSet: LIMIT, limit: LIMIT }), - memoryHighPercent: 85, + now: time.now, heldByPerson: (botId) => botId === "held", }); - const { profiles } = made; try { await profiles.page("held"); await profiles.page("other"); - // Both are live before the third arrives: launching "other" closed nothing it could, because - // the only other browser was the one a person holds. - expect(made.told).toEqual([]); + await profiles.page("third"); + time.passes(10 * MINUTE); await profiles.page("asking"); - // "other" goes; "held" is a person's, and "asking" is the launch itself. - expect(made.told).toEqual(["other"]); - expect(profiles.isLive("held")).toBe(true); - expect(profiles.isLive("asking")).toBe(true); + // "held" is a person's to work in and "third" was the most recently used: "other" goes. + expect(told).toEqual(["other"]); - // The idle sweep's tick reads the same and closes nothing more: what is left is held, or the - // newest. + // The sweep's tick reads full again. "third" is quiet and goes; "held" and the newest stay. await profiles.sweepIdleNow(); - expect(made.told).toEqual(["other"]); + expect(told).toEqual(["other", "third"]); + expect(profiles.isLive("held")).toBe(true); + expect(profiles.isLive("asking")).toBe(true); expect(profiles.liveCount()).toBe(2); } finally { await profiles.closeAll(); @@ -171,6 +281,43 @@ describe.skipIf(!asked)("closing browsers when memory runs high", () => { }, LAUNCH_TIMEOUT_MS, ); + + test( + "a person's own browser is never closed for memory, even the one used longest ago", + async () => { + // Their Recorder runs in it, so closing it would end what they were recording without a word. + const { findRecorder } = await import("../src/recorder"); + const found = await findRecorder( + join(import.meta.dir, "fixtures", "recorder"), + ); + if (!found.ok) throw new Error(found.error); + const container = containerOf(30); + const time = clock(); + const { profiles, told } = await profilesFor("memory-person", { + memory: container.memory, + now: time.now, + }); + try { + container.holds( + await profiles.page(PERSON, { open: { recorder: found.recorder } }), + ); + container.holds(await profiles.page("bot-a")); + container.holds(await profiles.page("bot-b")); + time.passes(10 * MINUTE); + + container.holds(await profiles.page("bot-c")); + + // The person's browser is the least recently used by ten minutes and stays; the quieter of + // the two Bots goes in its place. + expect(told).toEqual(["bot-a"]); + expect(profiles.isLive(PERSON)).toBe(true); + expect(profiles.person(PERSON)).toBeDefined(); + } finally { + await profiles.closeAll(); + } + }, + LAUNCH_TIMEOUT_MS, + ); }); describe.skipIf(!asked)("closing a Bot's idle background tabs", () => { diff --git a/agent-computer/tests/container-memory.test.ts b/agent-computer/tests/container-memory.test.ts index 4e8827351c..4cfa17f53c 100644 --- a/agent-computer/tests/container-memory.test.ts +++ b/agent-computer/tests/container-memory.test.ts @@ -2,7 +2,12 @@ import { describe, expect, test } from "bun:test"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { memoryRunsHigh, readContainerMemory } from "../src/container-memory"; +import { + memoryAtOrAbove, + memoryRunsHigh, + memoryTargetPercent, + readContainerMemory, +} from "../src/container-memory"; /** * How much of its memory a computer is using, from the files a real container wrote. @@ -119,3 +124,26 @@ describe("whether memory runs high", () => { expect(memoryRunsHigh({ workingSet: LIMIT, limit: LIMIT }, 0)).toBe(false); }); }); + +describe("where closing for memory stops", () => { + const LIMIT = 1_000; + + test("ten points under where it starts, so 75 at the default 85", () => { + expect(memoryTargetPercent(85)).toBe(75); + expect(memoryTargetPercent(60)).toBe(50); + }); + + test("between the two lines nothing starts, and a pass that started goes on", () => { + // 80% does not start a pass at 85, and a pass already closing at 85 does not stop at 80: it + // closes on to under 75, so the next launch does not cross 85 again straight away. + const at80 = { workingSet: 800, limit: LIMIT }; + expect(memoryRunsHigh(at80, 85)).toBe(false); + expect(memoryAtOrAbove(at80, memoryTargetPercent(85))).toBe(true); + expect( + memoryAtOrAbove( + { workingSet: 749, limit: LIMIT }, + memoryTargetPercent(85), + ), + ).toBe(false); + }); +}); diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index a9ac345cd7..37064ca658 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -404,10 +404,11 @@ computers: # the process default, which is a handful and thirty minutes. maxBrowsers: "" browserIdleMs: "" - # At what share of the pod's memory limit the computer starts closing browsers, least recently used - # first, so a few heavy pages close one Bot's browser rather than the OOM killer taking the pod and - # every browser on it. Measured as the kubelet measures it (the working set). Empty means 85; 0 - # switches it off. + # At what share of the pod's memory limit the computer starts closing browsers nobody is using, + # least recently used first, down to ten points under it. Measured as the kubelet measures it (the + # working set), before each launch and once a minute: it stops idle browsers filling the pod one + # launch at a time, and a page that grows between two checks can still run the pod out of memory. + # Empty means 85; 0 switches it off. memoryHighPercent: "" # How long a Bot's background tab (one it left behind, such as the page a sign-in popup opened from) # is kept untouched before it is closed. The page the Bot is on is never closed. Empty means ten diff --git a/docker-compose.yml b/docker-compose.yml index c7a8ec8da9..889f3b8744 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -270,9 +270,10 @@ services: # minutes; closing a browser costs only a relaunch, because the profile is on the volume below. COMPUTER_MAX_BROWSERS: ${COMPUTER_MAX_BROWSERS:-} COMPUTER_BROWSER_IDLE_MS: ${COMPUTER_BROWSER_IDLE_MS:-} - # At what share of the container's memory it starts closing browsers, least recently used first - # (default 85), and how long a Bot's background tab may sit untouched (default ten minutes). A - # container with no --memory is measured against the machine's memory. 0 switches either off. + # At what share of the container's memory it starts closing browsers nobody is using, least + # recently used first, down to ten points under (default 85), and how long a Bot's background + # tab may sit untouched (default ten minutes). A container with no --memory is measured against + # the machine's memory. 0 switches either off. COMPUTER_MEMORY_HIGH_PERCENT: ${COMPUTER_MEMORY_HIGH_PERCENT:-} COMPUTER_TAB_IDLE_MS: ${COMPUTER_TAB_IDLE_MS:-} # Every Bot without a computer of its own reaches this one, so each gets its own folder of diff --git a/docker/s6/s6-rc.d/computer/run b/docker/s6/s6-rc.d/computer/run index 7d7614597b..5ea54a4a40 100755 --- a/docker/s6/s6-rc.d/computer/run +++ b/docker/s6/s6-rc.d/computer/run @@ -12,6 +12,12 @@ # # `with-contenv` is not decoration. Without it s6 starts a service with none of the container's # environment, and the failure is a config error naming a variable that is plainly set. +# +# COMPUTER_MEMORY_HIGH_PERCENT stays on here, and here it counts everything in this container: the +# database and the API server as well as the browsers, because the cgroup the computer reads is the +# container's. That is deliberate. The kernel's out-of-memory kill counts all of it too and can take +# the database down, so closing a browser nobody is using, before memory gets there, is how the +# database is protected. Set it to 0 to leave memory to the kernel. set -eu if [ "${EMBEDDED_COMPUTER:-on}" != "on" ]; then exec /bin/true diff --git a/docs/configuration.md b/docs/configuration.md index b42942ce2c..350cedd216 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -651,7 +651,7 @@ is still the record. | `COMPUTER_MAX_BROWSERS` | How many Bots may hold a running browser at once. `8` by default; the least recently used is closed past it. A person's browser in a native window (OpenBot Desktop) counts toward it and is closed only when no Bot's browser is left to close; a person's cloud browser is ranked by use like a Bot's. | | `COMPUTER_SEPARATE_WORKSPACES` | `on` gives each Bot its own folder of files on a computer many Bots share, with its shell starting there. On for Helm's `shared` computer and the docker-compose `agent-computer`; off elsewhere. Files saved before it was on stay at the root of the volume. Commands can still read other folders: use `computers.mode: sandbox` to keep people apart. | | `COMPUTER_BROWSER_IDLE_MS` | How long an untouched browser is kept. 30 minutes by default; `0` keeps them resident. A person's cloud browser takes the same rule: the Browser page touches it every second while it is open, so it closes once nobody is on that page. A person's native window (OpenBot Desktop) is never closed for being idle. | -| `COMPUTER_MEMORY_HIGH_PERCENT` | At what share of its memory limit a computer closes browsers, least recently used first and one at a time, until it is back under. `85` by default; `0` switches it off. Memory is the container's working set, as the kubelet reads it (usage less the inactive page cache), checked before every launch and every minute. A container with no limit is measured against the machine's memory; a computer outside a container (OpenBot Desktop's) closes nothing for memory. Never closed for memory: the browser being launched, the most recently used one, and a Bot's whose wheel a person holds or has been asked to take. A person's native window goes last, as at the cap. Each close logs `computer-browser-closed-for-memory` with the working set and the limit in bytes. | +| `COMPUTER_MEMORY_HIGH_PERCENT` | At what share of its memory limit a computer starts closing browsers nobody is using, least recently used first and one at a time, until it is ten points under that share (75% at the default). `85` by default; `0` switches it off. Memory is the container's working set, as the kubelet reads it (usage less the inactive page cache), checked before every launch and every minute, one pass at a time however many launches arrive together. It is a check, not a guarantee: a page that grows between two checks can still run the container out of memory. A container with no limit is measured against the machine's memory; a computer outside a container (OpenBot Desktop's) closes nothing for memory. Never closed for memory: the browser being launched, the most recently used one, any used in the last minute, a person's own browser (cloud or native), and a Bot's whose wheel a person holds or has been asked to take. In the all-in-one image the share counts the database and the API server as well as the browsers, on purpose: the kernel's out-of-memory kill counts them too and can take the database down, and closing a browser first is how the database is protected. Each close logs `computer-browser-closed-for-memory` with the working set and the limit in bytes. | | `COMPUTER_TAB_IDLE_MS` | How long a Bot's background tab may sit untouched before it is closed. 10 minutes by default; `0` keeps them. A Bot is always on its newest page, so an older tab (the page a sign-in popup opened from, a link opened in a new tab) is one it cannot go back to. Navigating, being acted on, or being in front counts as use. Never closed: the page the Bot is on, a person's own browser, and any tab of a Bot whose wheel a person holds. | | `OPENBOT_RECORDER_DIR` | The built OpenBot Recorder (`extension/dist` after `bun run build`), absolute. Every person's browser the computer opens has it loaded and connected; without it, or pointed at anything that is not a built Recorder, the computer still serves Bots and refuses to open a person's browser, saying why. The computer image sets it to the Recorder it carries. | | `COMPUTER_PEOPLE_ONLY` | `on` for a computer that runs people's own browsers and no Bot (OpenBot Desktop sets it on its host-side computer); it refuses every Bot. A person's browser runs only on a computer of its own: this one, or one started for it alone (`COMPUTER_BOT_ID`, as the supervisor and a Sandbox start each computer). The shared computer refuses it, and so does the server, whose Browser page says why, because a Bot's shell there could read the person's logins off the disk. |