fix(memory): bound INDEX_CACHE lifetime in long-running gateway daemons#86293
fix(memory): bound INDEX_CACHE lifetime in long-running gateway daemons#86293quengh wants to merge 3 commits into
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open: this is a focused fix for a real long-running gateway memory/resource leak with strong production soak proof, but the current patch has a cache-metadata race during close that can leave replacement managers outside future idle sweeps. Canonical path: Close this PR as superseded by #40389. So I’m closing this here and keeping the remaining discussion on #40389. Review detailsBest possible solution: Close this PR as superseded by #40389. Do we have a high-confidence way to reproduce the issue? Yes for the reported leak in source/proof: current main and the latest release only tear down the cache on explicit close paths, while the PR body gives production macOS Is this the best way to solve the issue? No, not yet: idle eviction is the right shape for bounding daemon lifetime resources, but the implementation must preserve replacement-manager metadata during old-manager close. The safer path is an identity-guarded cleanup plus a race regression test before maintainer approval of the default-on config contract. Security review: Security review cleared: No concrete security or supply-chain regression was found; the diff does not change CI, dependencies, package resolution, secrets, credentials, or external code execution paths. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 730fd1907fba. |
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Frosted Review Wisp Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
|
Thanks for the review. Picking option (2) from your maintainer options and pushing a doc/comment clarification commit ( Why option (2) is the right contractThe two new knobs are intentionally gateway-wide rather than per-agent:
Why I'm not narrowing the schemaThe zod The clarification commit instead:
If you'd prefer the schema-narrowing approach instead, I can do that as a follow-up commit — happy to defer to whichever shape maintainers prefer. The runtime behavior would not change either way. Soak update13.5h → 14.5h elapsed since I opened this PR. Sampler still running, RSS now |
|
Two follow-up commits address the findings from the previous review:
Soak update: 14.7h elapsed, RSS @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
The previous re-review picked up
PR head should be Soak update: 14.9h elapsed, RSS @clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
Cached MemoryIndexManager instances live in a module-level INDEX_CACHE that grows for the entire gateway daemon lifetime. Each instance owns a chokidar FSWatcher recursively watching the memory directory (under the default sync.watch=true), so every cache entry holds an FSEventWrap native handle plus one awaitWriteFinish file descriptor per touched memory/*.md file. Cached managers are torn down at CLI exit (openclaw#40389), but daemons never reach that exit path until restart. A production daemon (OpenClaw 2026.4.15) measured +1.6 GB RSS over 11 h and 612 accumulated FSEventWrap handles before crashing. This change adds a periodic runtime sweep that closes idle managers during normal daemon operation, so restart is no longer the only way to release these resources. Approach -------- ManagedCache: - Track lastAccessAt per cache key (refreshed on hit and on initial set). - closeIdleManagedCacheEntries walks lastAccessAt, closes stale entries, and isolates close() errors so one bad entry does not stop the rest. Reuses the per-manager close() introduced in openclaw#40389 — no new teardown code path. In-flight protection (always-on safety net): - inflightCount map keyed the same way as cache / lastAccessAt. - acquireManagedCacheKey() increments the count, refreshes lastAccessAt, and returns an idempotent release() function. - isManagedCacheKeyBusy() reports whether the count is > 0. - closeIdleManagedCacheEntries consults isManagedCacheKeyBusy in both the survey loop and immediately before close(); busy entries are skipped, counted, and surfaced via onSkipBusy / skippedBusy. - A revalidation gate (just before close) also re-checks lastAccessAt freshness and cache identity, catching entries that became active or were swapped during the scan-to-close gap. Reported as skippedRevalidated. MemoryIndexManager: - Protected withBusy(fn) helper wraps acquire/release in try/finally. - Guards sync(), search(), and ensureProviderInitialized() so the busy ref is held across provider init as well as the manager work itself. - close() and closeMemoryIndexManagersForAgent clear any residual inflightCount entry alongside the cache slot. release() is idempotent so a release that fires after close() is a no-op. Gateway sidecar: - scheduleMemoryIndexIdleEvict registers a gateway-lifetime sidecar that calls closeIdleMemorySearchManagers every idleEvictScanMs (default 5 min), evicting managers idle for idleEvictMs (default 15 min). Setting either to 0 disables the sweep entirely. Configuration ------------- Two new optional fields under agents.defaults.memorySearch.sync: - idleEvictMs (default 900000): idle TTL before a manager is eligible for eviction. 0 disables. - idleEvictScanMs (default 300000): sweep cadence. 0 disables. Documented in the config schema (zod), the MemorySearchConfig type, and the UI field labels map. Validation ---------- Live soak on the production daemon (OpenClaw 2026.4.15 + this patch backported, 13 h+ continuous, sampling every 10 min): | Metric | Without patch (11 h) | With patch (13 h soak) | |-----------------|-------------------------|-------------------------------| | FSEventWrap | 612 native handles | bounded, returns to ~3 baseline | | memory_fd | 488 (chokidar holds) | 0 between sweeps | | RSS growth rate | ~1 GB/h | ~150 MB/h (~7x improvement) | | Outcome at 13 h | OOM within 4 to 6 h | 3.25 GB stable, no OOM | Sidecar observed behavior: - fd accumulates during memory_search activity (up to ~600 handles) - next sweep evicts idle managers, fd returns to baseline within 10 min - skippedBusy and skippedRevalidated stayed at 0 throughout the soak — the workload burst rate did not exercise the deferral paths, but the unit tests cover them deterministically Tests ----- - extensions/memory-core/src/memory/manager-cache.test.ts: 21 tests (was 11). New coverage: acquireManagedCacheKey, isManagedCacheKeyBusy, busy deferral, refcount, lastAccessAt refresh, inflightCount cleanup, revalidation gate for cache-hit / busy / identity-swap / slot- disappearance gaps. - extensions/memory-core/src/memory/manager.idle-gc.test.ts: 10 tests (was 5). New coverage: in-flight protection at manager-level entry points (search, sync, ensureProviderInitialized). - extensions/memory-core/: 718/718 across 57 files. - src/gateway/server-startup-post-attach.test.ts: 41 tests pass. - src/agents/memory-search.test.ts: 27 tests pass. - pnpm run lint, tsgo:core, tsgo:extensions, tsgo:test, build all green. Risk / Compatibility -------------------- - Default idle threshold (15 min) is conservative — comfortably outlives typical bursts of memory_search calls within a session. - Worst-case dwell is idleMs + scanMs + max in-flight op duration (was unbounded; idleMs + scanMs if all ops are short). - Short-lived CLI invocations are unchanged: closeAllMemoryIndexManagers remains the exit path from openclaw#40389, and it clears inflightCount alongside the rest of the cache. - Public MemoryPluginRuntime.closeIdleMemorySearchManagers return type grows two additive fields (skippedBusy, skippedRevalidated). Existing callers reading evicted / remaining continue to work unchanged via TypeScript structural typing. - No new dependencies, no schema migration, no protocol change. Out of scope ------------ - Does NOT change sync.watch default value (openclaw#71335 tracks that separately). - Does NOT refactor the watcher to be singleton-per-workspace (much larger architectural change). - Does NOT introduce an opt-out for the in-flight protection — it's a safety net that should always be on. The only configurable knobs are the sweep cadence and idle threshold. Refs openclaw#71335, openclaw#40389.
The `idleEvictMs` and `idleEvictScanMs` knobs are resolved from `agents.defaults.memorySearch.sync` only. Spell this out in three places where an integrator might look first: - `MemorySearchConfig.sync.idleEvictMs` / `idleEvictScanMs` TS docs: state explicitly that the underlying `INDEX_CACHE` is a module-level singleton shared by every agent, and that per-agent values are accepted by the (shared) schema but ignored at runtime. - `MemorySearchSchema` zod comments: same contract repeated where someone reading the schema would expect to see it, pointing back at the TS doc for the full reasoning. - `agents.defaults.memorySearch.sync.idleEvict*` UI labels: tag the field name with `gateway-wide` so the UI reflects the scope. - `resolveMemoryIndexIdleEvictPolicy` in `server-startup-post-attach.ts`: document why this reader looks at `agents.defaults` only and what the design alternative would have to solve (no meaningful way to honour a per-agent threshold while another agent in the same gateway wants the same manager kept alive, since the cache itself is process-wide). No behavior change. Same 18 files in this branch otherwise; this commit only edits docstrings, schema comments, label strings, and one in-source comment block. Tests untouched and still passing (41/41 in `server-startup-post-attach.test.ts`).
Addresses the P3 review finding by surfacing the two new gateway-wide config keys in the places users typically discover memory settings: - `src/config/schema.help.ts`: add `FIELD_HELP` entries for `agents.defaults.memorySearch.sync.idleEvictMs` and `idleEvictScanMs`, explaining the unit, default, `0`-disables behavior, and the gateway-wide scope (default-only; per-agent overrides ignored at runtime). - `docs/reference/memory-config.md`: add a new "Idle eviction for cached memory managers" section after the inline embedding timeout section. Documents the leak it addresses, both knobs as `<ParamField>` blocks with defaults, and the in-flight protection behavior so operators understand why an aggressive `idleEvictMs` is safe. Same gateway-wide-only contract as the previous docs commit (`287edf77475`). No runtime behavior change. Tests: 1493/1493 pass in `src/config/` (full config test surface), including `schema.help` coverage and `runtime-config` schema tests.
5307802 to
76c1dbb
Compare
|
Rebased onto current Branch is now 0 commits behind main, 3 ahead. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
ClawSweeper applied the proposed close for this PR.
|
Real behavior proof
Behavior addressed: Long-running OpenClaw gateway daemons accumulate chokidar
FSEventWrapnative handles andmemory/*.mdfile descriptors for the entire process lifetime. RSS grows ~1 GB/h until OOM, typically within 4–6 h on memory-heavy workloads. Root cause:MemoryIndexManagerinstances live in the module-levelINDEX_CACHEand are only torn down at process exit (#40389), which long-running daemons never reach until restart.Real environment tested: Production gateway daemon on macOS 15.7.4 (Apple Silicon), OpenClaw
2026.4.15base, default config (agents.defaults.memorySearch.sync.watch=true), Nodev24.13.0. Patch applied at runtime by replacing the installeddist/directory with a build of this branch (fix/memory-index-manager-runtime-eviction), thenlaunchctl kickstart -k gui/501/ai.openclaw.gatewayto restart the daemon onto the patched dist.Exact steps or command run after this patch: see code block below.
Evidence after fix: real terminal output from the soaked production gateway (PID 93144, 14 h elapsed at PR submission).
Process state at +14 h on patched daemon (started 2026-05-24 20:37 +0800):
fd breakdown at steady state between sweeps:
Sampler CSV excerpts (
/tmp/soak-v2-samples.csv, 78 data points, 10 min interval):For comparison, before the patch (2026-05-23, same daemon, 11 h uptime, no sidecar) heap snapshot diff showed:
and
lsofof that pre-patch daemon showed488chokidarawaitWriteFinishread fds on~/.openclaw/workspace/memory/.Observed result after fix:
memory_fd(chokidar read fds onmemory/*.md) oscillates between0baseline and ~557 during bursts, returning to0within one sweep interval — without the patch, this value grew monotonically to 488+ and never returned to baseline.kqueuestays pinned at exactly3for the entire 14 h soak — without the patch,FSEventWrapinstances accumulated by the hundreds (+612 in the pre-patch 11 h heap diff). RSS rose ~150 MB/h (from 1.37 GB at +15 min to 3.16 GB at +14 h), well under OOM — without the patch the same workload hit OOM at ~4.7 GB within 4–6 h. Sidecar log lines confirm the sweep is running and reportingmemory index manager idle eviction: evicted N (deferred M busy, K revalidated) (remaining R). 31 unit tests cover the in-flight protection deferral and revalidation paths deterministically (the live soak never hit those paths because our workload's burst rate is slower than the 15 min idle threshold, but the guards are wired and tested).What was not tested: Windows / Linux daemons (only macOS validated live; CI covers cross-platform builds, but no live soak on those platforms yet). Multi-agent workloads exercising > 50
MemoryIndexManagerinstances simultaneously (our production setup has ~3 active managers). Workloads where individualsync()batches exceed the 15 min idle threshold (would exercise the busy-defer path live; covered by unit tests but not by soak). 24 h+ continuous soak (in progress; 14 h at PR submission, will post 24 h and 48 h checkpoints as PR comments).Out-of-scope follow-ups
sync.watchdefault fromtruetofalse(tracked in Feature: sync.watch should default to false in gateway mode #71335)Problem
Cached
MemoryIndexManagerinstances live in a module-levelINDEX_CACHEthat grows for the entire gateway daemon lifetime. Each instance owns a chokidarFSWatcherrecursively watching the memory directory (under the defaultsync.watch=true, see #71335), so every cache entry holds anFSEventWrapnative handle plus oneawaitWriteFinishfile descriptor per touchedmemory/*.mdfile.Cached managers are torn down at CLI exit (#40389), but daemons never reach that exit path until restart. This PR adds a periodic runtime sweep that bounds cache lifetime during normal daemon operation, so restart is no longer the only way to release these resources.
Measured impact (production daemon, OpenClaw 2026.4.15 + this patch)
Sampling: 78 data points at 10 min interval, soak still running at PR submission time (will update when the soak passes the 24 h and 48 h marks). Sweep cadence: 5 min scan, 15 min idle threshold (both configurable).
Fix
Extend
ManagedCachewith idle tracking and add a gateway-lifetime sidecar:ManagedCache.lastAccessAtis refreshed on every cache hit and on initial set.closeIdleManagedCacheEntrieswalkslastAccessAt, closes stale entries, and isolatesclose()errors so one bad entry does not stop the rest. It reuses the per-managerclose()method introduced in Fix one-shot exit hangs by tearing down cached memory managers #40389 — no new teardown code path.closeIdleMemoryIndexManagers(inmanager.ts) layers in the embedding-probe-cache cleanup that the manager would otherwise miss.scheduleMemoryIndexIdleEvict(inserver-startup-post-attach.ts) registers a gateway-lifetime sidecar that runs the sweep everyidleEvictScanMs(default 5 min), evicting managers idle foridleEvictMs(default 15 min). Setting either to0disables the sweep entirely.In-flight protection
ManagedCachetracks aninflightCountmap keyed the same way ascache/lastAccessAt:acquireManagedCacheKey(cache, key)increments the count, refresheslastAccessAt, and returns an idempotentrelease()function.isManagedCacheKeyBusy(cache, key)reports whether the count is > 0.closeIdleManagedCacheEntriesconsultsisManagedCacheKeyBusyduring both the survey loop and immediately beforeclose(). Busy entries are skipped, counted, and surfaced viaonSkipBusy/skippedBusy.lastAccessAtfreshness and cache identity, catching entries that became active or were swapped during the scan-to-close gap. These are reported asskippedRevalidated.In
MemoryIndexManager, a protectedwithBusy(fn)helper wraps the acquire/release pair in try/finally. It guards:sync(...)— covers batch reindex, targeted session sync, full sync. The refcount is released in the same.finally()that nullsthis.syncing, so they unwind together.search(...)— protects the embedding/vector/FTS work plus the inlinesync({ reason: "search", force: true })bootstrap that runs on the first call after process start.ensureProviderInitialized(...)— coversprobeVectorAvailability,probeEmbeddingAvailability, and the lazy provider hookup invoked bysync/search.close()andcloseMemoryIndexManagersForAgentclear any residualinflightCountentry alongside the cache slot. Therelease()returned byacquireManagedCacheKeyis idempotent, so a release that fires afterclose()is a no-op.Sidecar log lines now report both deferral classes:
Configuration
Two new optional knobs under
agents.defaults.memorySearch.sync:idleEvictMs0disables.idleEvictScanMs0disables.Both fields are documented in the zod schema (
src/config/zod-schema.agent-runtime.ts), the TypeScript type definitions (src/config/types.tools.ts), and the UI field labels map (src/config/schema.labels.ts).Validation
Live soak (production daemon)
sync.watch=true)lsof-derived fd counts +ps-derived RSS, 10 min intervalSidecar observed behavior during the soak:
memory_searchactivity (up to ~600 handles)skippedBusy=0andskippedRevalidated=0throughout — confirms the in-flight and revalidation guards are wired correctly but our workload's burst rate did not exercise them (the unit tests below do)Tests
extensions/memory-core/src/memory/manager-cache.test.ts— 21 tests (was 11). New coverage:acquireManagedCacheKey/isManagedCacheKeyBusy/ busy deferral / refcount /lastAccessAtrefresh /inflightCountcleanup / revalidation gate for cache-hit, busy, identity-swap, and slot-disappearance gaps.extensions/memory-core/src/memory/manager.idle-gc.test.ts— 10 tests (was 5). New coverage: in-flight protection at manager-level entry points (search,sync,ensureProviderInitialized).extensions/memory-core/— 718/718 across 57 files.src/gateway/server-startup-post-attach.test.ts— 41 tests pass.src/agents/memory-search.test.ts— 27 tests pass.pnpm run lint,pnpm run tsgo:core,pnpm run tsgo:extensions,pnpm run tsgo:test, andpnpm run buildall green.Risk / Compatibility
memory_searchcalls within a session.idleMs + scanMs + max in-flight op duration(was unbounded;idleMs + scanMsif all ops are short).closeAllMemoryIndexManagersis still the exit path from Fix one-shot exit hangs by tearing down cached memory managers #40389, and it clearsinflightCountalongside the rest of the cache.MemoryPluginRuntime.closeIdleMemorySearchManagersreturn type grows two additive fields (skippedBusy,skippedRevalidated). Existing callers readingevicted/remainingcontinue to work unchanged via TypeScript structural typing.Out of scope
sync.watchdefault value (Feature: sync.watch should default to false in gateway mode #71335 tracks that separately).Lifecycle phase comparison vs #40389 (for reviewers)
finally)idleEvictScanMsisBusy+withBusy+ revalidation gateclose()close()in the sweepThe two are layered: #40389 handles the moment of exit; this PR handles the time between starts and exits, which is where long-lived daemons spend ~100% of their wall clock.
Refs #71335, #40389.
Raw production data
Real terminal output from the soaked production gateway (PID 93144 at the time of writing, 14 h elapsed, OpenClaw
2026.4.15base + this patch backported as runtime dist replacement).Before the patch (baseline measured 2026-05-23, no patch, 11 h uptime)
Heap snapshot delta over 11 h (
chrome devtools heap snapshot):lsof -p <gateway>count by location (no patch, 11 h):RSS over 11 h grew from ~700 MB baseline to ~2.3 GB (+1.6 GB, ~150 MB/h);
process subsequently OOM'd at ~4.7 GB.
After the patch (live soak in progress, 14 h elapsed at PR submission)
pssnapshot at +14 h:lsoffd breakdown at +14 h (steady state between sweeps):Sidecar in action — sampler CSV excerpts (
/tmp/soak-v2-samples.csv, 78 lines, 10 min interval):Visible behavior in the CSV:
fd_totalswings between baseline~40and burst~600and reliably returns to baseline within one sweep interval (10 min in this run)memory_fd(chokidarawaitWriteFinishread fds) stays at0between bursts — they are released byclose(), not just dropped from the JS heapkqueuestays at exactly3throughout (native kevent handles) — confirmsFSEventWrapis being destroyed, not orphanedrss_kbrises slowly (~150 MB/h) but does not approach OOM territory; without the patch the same workload OOM'd within 4–6 hRaw CSV (83 lines) and any additional intervals available on request.