Skip to content

fix: GC for debug dump dirs, launcher temp logs, and bili.log.old - #277

Open
ranxianglei wants to merge 1 commit into
masterfrom
2026-08-27_gc-debug-dumps-logs
Open

fix: GC for debug dump dirs, launcher temp logs, and bili.log.old#277
ranxianglei wants to merge 1 commit into
masterfrom
2026-08-27_gc-debug-dumps-logs

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Part B of #255 (split from PR #274, which covers logs only). Closes #275.

Previously nothing was ever cleaned up:

  • ~/.local/state/billion-context/dumps/ and raw/ were write-only and grew without bound under --debug;
  • launcher temp logs bili-proxy-${port}.log (src/launcher.ts) were only process-group-killed, never unlinked;
  • bili.log 10MB rotation produced bili.log.old, but the .old was never deleted, so old generations accumulated forever.

Changes

  • New src/state-gc.tsgcDebugDir(dir, { maxFiles?, maxBytes? }): FIFO garbage collection by mtime with two bounds (default 500 files / 512 MB) and a per-directory 30 s throttle so the sweep stays off the hot path. Wired into every debug-dump write site in src/server.ts (raw/ INCOMING, dumps/ req, raw/ REQ/RES, and dumpStreamToFile).
  • src/logger.ts — rotation now deletes the previous .old generation before renaming, keeping at most one .old. This also fixes rotation on Windows, where renameSync cannot overwrite an existing target.
  • src/launcher.tsstopProxy now unlinks the owned proxy temp log; a new gcStaleProxyLogs() sweeps os.tmpdir() for bili-proxy-*.log older than 24 h (a live proxy rewrites its log on every request, so anything older is dead) and is called at the start of ensureProxyRunning to reap orphans left by crashed/killed launchers.

Tests

  • tests/state-gc.test.ts — count/size trimming, throttle, missing dir, subdirectory handling.
  • tests/logger-rotate.test.ts — rotation keeps at most one .old generation.
  • tests/launcher.test.tsstopProxy unlinks the temp log; gcStaleProxyLogs removes only stale bili-proxy-*.log files.

Full suite green: typecheck, 666 tests, build.

@github-actions

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-08-27_gc-debug-dumps-logs (13c0a46)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-277

Each push to this PR publishes a new version under the pr-277 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr277.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

[bot] Reviewed PR #277 (commit 13c0a46) on a clean checkout. LGTM — verified green, no blockers.

Verification (ran locally on the PR branch):

  • npm run typecheck — pass
  • npm test — 666/666 pass (incl. the 11 new tests: 6 state-gc, 1 logger-rotate, 2 launcher)
  • npm run build — success
  • package.json / package-lock.json untouched, per AGENTS.md

What I checked in the diff:

  • src/state-gc.ts — FIFO-by-mtime trim to both caps (500 files / 512 MB), deterministic tiebreak (mtime → name), 30 s per-dir throttle keeps it off the hot path, best-effort error handling throughout, loggerLog (no console.error), injectable nowMs for tests. No as any / @ts-ignore.
  • src/logger.tsunlinkSync(file + ".old") before renameSync is correctly placed inside the existing try/catch; fixes the Windows rename-overwrite failure and caps generations at one .old.
  • src/launcher.tsstopProxy unlinks the owned logPath (field already on ProxyHandle); gcStaleProxyLogs is prefix-scoped (bili-proxy-*.log), file-only (skips dirs), 24 h threshold, and is called only from ensureProxyRunning (2 call sites, launch-time) so the tmpdir readdir is not on any hot path.
  • src/server.ts — all four dump write sites wired (raw INCOMING, dumps req, raw REQ/RES, dumpStreamToFile via ws.end callback). I grepped every writeFileSync/createWriteStream in the server-side modules: no unbounded write site is missed. The let rawDir = "" hoist is minimal and the "" fallback is harmless (readdirSync("") → ENOENT → 0). Dump dirs are all flat-file, so ignoring subdirectories is safe.

Minor notes (non-blocking, no action required):

  1. The 24 h rationale ("a live proxy rewrites its log on every request") has one edge: a live but idle proxy (>24 h without a request) could get its log unlinked by the sweep. On POSIX the child keeps writing to the unlinked inode (silent log loss, no crash); on Windows the unlink fails on the open file (caught, retried next launch). Worst case is lost logs for a long-idle proxy — acceptable tradeoff, just noting it.
  2. gcDebugDir records the throttle timestamp even when readdir fails (missing dir), so a dir created within 30 s gets its first sweep delayed by up to 30 s. Harmless.
  3. Merge-order note: this PR and fix: mask credentials and non-public API endpoints in proxy logs (#255) #274 both touch src/server.ts in adjacent regions (~line 1634 insert here vs ~line 1639 modify there). Should auto-merge, but if fix: mask credentials and non-public API endpoints in proxy logs (#255) #274 lands first, expect a small rebase.

Ready to merge whenever you are.

🤖 ework agent · vllm-qwen/qwen3.8-27b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

debug 落盘:请求体明文 dump 默认开启 + dump 头过滤漏 cookie/set-cookie

1 participant