Repository navigation
🤖 fix: compact devtools.jsonl to the retained DevTools window - #6890
ibetitsmike wants to merge 3 commits into
Conversation
devtools.jsonl was append-only: memory kept only the newest 100 runs or 64 MiB per workspace, but the file kept every run ever logged, so files reached several GB and each first load after a restart replayed all of it. Once more than twice the byte budget of the file is lines that replay no longer keeps (evicted runs, superseded step updates), DevToolsService now rewrites it as the retained runs and their merged steps (temp file plus rename, inside the per-workspace write queue). A load that finds an oversized file compacts it once. A failed rewrite keeps the full file and waits for another two budgets of growth before it tries again; a partial replay never compacts. The debug CLI's log reader now also takes usage, errors and wire facts from step rows, where a compacted log keeps them. With the default budget and 600 one-MiB steps, the file peaked at 190 MiB instead of 600 MiB; the append that compacts took 252 ms (mean 4 ms).
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb278af372
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Compaction rewrote the log from this process's memory. That erased rows another backend appended since this one loaded, wrote entries whose appends were still queued twice, and grouped steps by run, which changed the log order the replay audit reads. It now replays the file inside the session lock and writes what that replay keeps, with steps in log order and each run before its first step.
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba76b67dc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…writes Compaction replayed and rewrote the whole log while it held the session lock. That lock also guards the workspace's other session files, and other processes give up on it after 2 s, so a slow rewrite could make another backend drop the row it was appending. The replay and the rewrite now run outside the lock. Under it, the rewrite replaces the log only if the log is unchanged since the replay started, and rewrites that a crashed or overtaken backend left are removed. The temporary file name now includes the process id, because two backends can rewrite the same log at once. Each rewritten line is written in a loop until all of its bytes are out, because a nearly full disk can accept part of a write without an error.
|
@codex review Please take another look. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch has not been deployed
1 participant
Summary
With debug logging on,
devtools.jsonlgrows with every run and step ever logged, while the DevTools panel replays only a bounded recent window (64 MiB per workspace by default). This PR rewrites the file down to that window once more than twice the budget of it is lines that replay no longer keeps, so the file stays near the window instead of growing without bound.Background
#4260 bounded what replay keeps in memory and left the file untouched ("compaction is a follow-up"). Debug logs had reached 25 GiB on a dev machine.
Implementation
DevToolsServicetracks the file length (fileBytes, in the same units as the retention bytes) during replay and after each append.devtools.jsonl.compacting-<pid>, looping until every byte of each line is written.commitToSessionFileUnlessRemoved, so a removed workspace's directory is not recreated), the rewrite replaces the log only if the log is unchanged since the replay started, and rewrites left by crashed or overtaken backends are removed.readDevtoolsLog(debug CLIreplay-verifyandcache-audit) now also reads usage, error, and prompt-cache facts from step rows, because compaction mergesstep-updaterows into them.Validation
devToolsService.test.tsand a step-row layout case inreplayVerify.test.ts. Each fails with its trigger or guard removed, or on the commit before its fix.Risks
Review record