Repository navigation
perf(sessions): bound flat Codex directory entry traversal - #4857
ScriptedAlchemy wants to merge 15 commits into
Conversation
Refs #4756 A flat archived directory with many .zst/non-JSONL entries previously consumed its entire ReadDir iterator in one structural scan turn, outside the directory-work budget. Charge directory entries independently of child directory traversal while retaining the iterator across turns. Add a real archived-directory regression requiring the first scan to yield in the directory phase, then converge on exactly the eligible JSONL path. Partial performance repair only; durable restart continuation and affected host CPU/write-latency acceptance remain outstanding.
|
Refs #4756 Keep unsettled directory name witnesses scoped to traversable child buckets and JSONL candidate paths. Compressed rollouts and other ignored file churn must not independently invalidate the candidate witness. Cover ignored additions and renames, candidate admission, and child bucket discovery with a real-filesystem regression.
Refs #4756 — follow-up Codex discovery witness repair (partial)Published Cause and change: Even after limiting structural Nonvacuous filesystem regression: 128 ignored Detached exact diff is one commit ahead, two files, +46/-1; independently checked the diff and branch head. Exact-head Bazel CI run 42769750119 was queued at inspection; there is no local checkout or Bazel executable, so no Rust test pass or incident-scale performance measurement is claimed. No Cargo used. This is not |
Refs #4756 Apply the two exact formatting hunks emitted by Bazel CI; preserve every behavioral test assertion.
|
Refs #4756 — CI repair published as The exact preceding-head Bazel run #42769750119 failed New exact-head hosted Bazel run #42775257282 is queued. No Rust test pass, durable mid-directory restart-continuation proof, or incident-scale CPU/SQLite write-latency acceptance is claimed. This remains |
Refs #4756 — eliminate eager full-archive name enumeration (partial)Published Root cause: the recently bounded structural Source fix: obtain the native directory identity only; retain an incremental, constant-memory, order-independent candidate-name digest on the same New real-filesystem regression: create 128 ignored Review: independent GitHub compare verifies one commit with only This specifically removes the eager unbounded name-scan before bounded initial discovery; idle unsettled-directory rechecks, durable mid-directory restart continuation, and measured 4-hour CPU / SQLite write latency acceptance are not yet proved. Keep #4756 open, retain |
Refs #4756 — exact-head CI compile and format repair (partial)Published Confirmed CI root causes: the preceding Bazel run #42781976950 produced two Changes: change both depth references to New exact-head Bazel CI #42898370855 is running. There is no local Bazel checkout in this run; this is not a claim of a passing Rust test. Still unresolved: durable mid-directory traversal resume across daemon restarts; end-to-end provider-separated CPU and SQLite bounded-write evidence on an incident-scale corpus. Keep #4756 open and this PR unmerged pending proof; do not use |
|
Refs #4756 — published CI compilation fix (partial only) Exact-head Bazel CI #42898370855 failed Linux fleet, Clippy and Windows because Published New exact-head Bazel CI #42904574809 is queued; no Rust test pass is claimed. The in-process structural and fingerprint work bounds remain partial: native |
Refs #4756 A budgeted directory iterator may span multiple ticks while new entries arrive. Verify its opening stat witness when it is exhausted and keep coverage incomplete after an observed change, then require a fresh sweep. Add a real filesystem regression for mid-scan mutation and convergence.
Refs #4756 The first sweep may already emit the newly arrived rollout even when its directory witness requires another pass. Assert complete source coverage across both the invalidated and stable sweeps.
Refs #4756 Retain the opening stat on the directory iterator itself so changes across structural memory windows still invalidate sweep completion, even when the prior chunk's directory list was released.
Refs #4756 Match the repository's multiline Rust formatting without changing the mutation fence or the real filesystem regression.
|
Refs #4756 — published a second correctness slice on this existing issue PR (not Newly identified race: an unfinished Codex Published four exact-expected-head, nonforce fast-forward commits directly to #4857, preserving its earlier flat-entry work and all peer branches:
I fetched and reviewed each published commit's actual GitHub patch: all changes are confined to This is a source-authority safety fix, not durable mid-directory resume across process restart. The original ~4h/~1.7-core import, 125–160ms writer latency and provider-separated restart continuity remain unproven, and #4793's independent SQLite/write-recovery lane is untouched. Keep PR unmerged and issue OPEN until relevant exact-head nonzero Bazel/host tests, independent review and master incident-scale acceptance. Kev/decision-model and mixed #4776 remain draft-only; release #4763 untouched; no production/infra/tags/credentials or other schedules changed. |
Refs #4756 An unsettled native change clock can coalesce directory mutations across a retained ReadDir walk. Matching opening and closing stat bytes are not enough to assert complete discovery. Require a settled closing witness before certifying the sweep; retain in-progress coverage until validation can establish a stable directory. Add a real bounded-read regression that aliases the native stat across a mid-scan JSONL arrival and rejects a false complete watermark.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Refs #4756 Revert an over-conservative unsettled-stat condition that could prevent completed fresh scans until the filesystem clock settled. Instead, recheck each retained file-phase directory identity at EOF so files arriving after the structural walk cannot be silently omitted from a complete frontier. Preserve the existing per-turn entry budget. Add a real filesystem regression inserting JSONL while file-phase ReadDir is suspended; full coverage must remain in progress.
|
Refs #4756 — published a focused Codex file-phase coverage correction on the existing issue PR. New source-level race: The bounded Published net change: Boundaries: An initial conservative coarse-clock check in No local checkout/Bazel is available and no Rust pass is claimed. Exact-head Bazel CI 42930379989 is queued; require actual nonzero |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Refs #4756 After a retained file iterator finishes under a coarse directory clock, reopen its eligible-name walk in bounded chunks and compare against the structural name fingerprint before granting complete coverage. Preserve Windows convergence without assuming directories have settled timestamps. Regress a late eligible rollout with an intentionally aliased coarse stat witness; keep all file and directory per-call budgets unchanged.
Refs #4756 An initial coarse-clock name fingerprint must be independently checked when the file iterator closes, even if its clock has since settled. Do not poison a temporarily suspended structural directory batch. Keep the aliasing regression deterministic across slow test hosts.
| } else if closing.is_some_and(|stamp| !stamp.settled) | ||
| && resume_directories.is_none() | ||
| { | ||
| // An unfinished structural iterator is | ||
| // resumed below; only its final pass may | ||
| // certify coverage without a name witness. | ||
| scan.complete = false; |
There was a problem hiding this comment.
🔴 Chunked archive misses newly added rollouts
When a rollout arrives during a chunked file walk with an aliased directory timestamp, resume_directories permits complete coverage. The resumed structural walk does not scan rollout files, so the missed rollout remains absent from the completed frontier.
Learn more
A retained directory iterator can be paused for a file-phase pass when directory retention fills. The file iterator can miss a newly added rollout whose directory timestamp aliases the original on a coarse filesystem. With resume_directories present, this branch leaves scan.complete true despite the missing entry-name witness. The subsequent resume transition clears the directory identity; the resumed structural iterator scans only for child directories and cannot add the omitted file or restore that identity's fingerprint. Completion can therefore persist without having observed the rollout.
Example: A flat archive fills the retained-directory budget while its structural ReadDir remains open. A .jsonl file arrives after the file reader has passed its position, with the same coarse native directory timestamp. The file reader omits it; resuming the structural reader does not emit it; the sweep still reports complete.
Recommended fix: Retain a verifiable name witness across chunk transitions and compare it against a fresh bounded listing after the structural iterator finishes, or mark coverage incomplete when a file walk ends unsettled without such a witness. Cover a chunked archive whose directory timestamp aliases a new .jsonl during the paused file walk.
Was this helpful? React with 👍 or 👎 to provide feedback.
Refs #4756 — partial Codex directory-work bound (not Fixes)
Root cause and focused correction
The initial
CodexScanPhase::Directoriesphase charged queued and discovered subdirectories against its per-turn budget, but did not charge ordinary directory entries read fromReadDir. A flat~/.codex/archived_sessionsdirectory containing many.zstor other non-directory names could therefore be fully enumerated by one scheduler call, before the separately bounded file-candidate phase. The reported corpus includes 1,385 Codex.zstentries; those entries are not accepted as JSONL transcripts by this reader, so their enumeration should not monopolize a tick.The source now counts every entry consumed from the structural
ReadDiriterator against its ownmax_files × (MAX_SCAN_DEPTH+2)work window. When the limit is reached, the existing process-owned iterator remains intact for the next call. Child-directory structural work and the candidate-file phase retain their independent existing limits. There is no new format, scheduler, durable storage, or SQL writer behavior.Nonvacuous behavioral regression
A real filesystem fixture creates 64 ignored
.zstentries and one admitted.jsonlin a flat archived Codex directory, then runsmax_files=1discovery. The first call must yield inside the directory phase with its live iterator retained, rather than consuming the archive and entering the file phase in one call; the previous implementation fails this assertion. Repeated bounded calls must converge on exactly the eligible JSONL file and earn the complete frontier, with an empty subsequent idle pass. No other test or assertion was removed.Verification and boundaries
AGENTS.mdand recursive master tree (no nested AGENTS.md), original issue and both issue comments, existing perf(sessions): bound projection queue-head scans per write window #4793/perf(sessions): isolate settled Codex scan optimization #4848/perf(sessions): skip settled Codex directory fingerprints #4815 owner boundaries and perf(sessions): bound projection queue-head scans per write window #4793 CI/reviews.835291a, directly parented to master2f9d659. Independently inspected the exact detached Git diff. Published to this separate issue branch byexpected_sha=2f9d659,force=false. perf(sessions): bound projection queue-head scans per write window #4793 and concurrent branches were not modified.bazel test //crates/tracedecay-sessions:unit_test --test_output=errors, repository rustfmt and Clippy, and applicable exact-head hosted CI before merge. No Cargo used.ReadDiriterator still does not survive daemon restart. This is not proof of incident-scale CPU reductions, 125–160 ms SQLite write latency elimination, or durable mid-directory restart continuation. Keep sessions: re-import after store reset holds ~1.7 cores for 4+ hours; profile_sessions writes take 125–160 ms; restart repeats the import #4756 open; noFixes #4756or merge hold removal.Kev/decision-model PRs and mixed #4776 remain DRAFT / never merge or auto-merge; release #4763 read-only. No production deployment, infrastructure, tags, credentials, Kody/Copilot/delegation, weakened CI, other owner edits or schedule changes.