Compress cold local rollouts#25089
Conversation
|
@codex review |
![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: b68133cdae
ℹ️ 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".
jif-oai
force-pushed
the
jif/rollout-materialize-compressed-appends
branch
from
2e9745e to
831e5e7
Compare
jif-oai
force-pushed
the
jif/local-rollout-compression-worker
branch
from
b68133c to
b996a74
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b996a74ada
ℹ️ 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".
jif-oai
force-pushed
the
jif/rollout-materialize-compressed-appends
branch
from
831e5e7 to
0825bb2
Compare
jif-oai
force-pushed
the
jif/local-rollout-compression-worker
branch
from
b996a74 to
d467d8e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d467d8ec0e
ℹ️ 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".
jif-oai
force-pushed
the
jif/rollout-materialize-compressed-appends
branch
from
66f796c to
acf9e8b
Compare
jif-oai
force-pushed
the
jif/local-rollout-compression-worker
branch
from
4617a0a to
d68c50d
Compare
## Why Local rollout compression needs a cold `.jsonl.zst` representation without letting compressed physical paths leak into append-mode writers. The unsafe case is resume or metadata update code successfully reading a compressed rollout and then appending raw JSONL bytes to the zstd file. This PR folds the former #25088 materialization slice into the read-support PR so the reader changes and append-safety invariant land together. ## What Changed - Teach rollout readers, discovery, listing, search, and ID lookup to understand compressed `.jsonl.zst` rollouts. - Keep `.jsonl` as the logical/stored rollout path while allowing read paths to open either plain or compressed storage. - Materialize compressed rollouts back to plain `.jsonl` before append-mode writes, including resume and direct metadata append paths. - Preserve compressed-file permissions when materializing back to plain JSONL. - Refresh thread-store resolved rollout paths after compatibility metadata writes so reconciliation follows the materialized file. - Avoid treating transient compression temp files as real rollout lookup results. ## Remaining Stack #25089 remains the separate worker PR. It is based directly on this PR and stays behind the disabled `local_thread_store_compression` feature flag. The worker still has a broader coordination question: a resume or metadata update can race with background compression while a plain file is being replaced by `.jsonl.zst`. This PR handles the read and materialize-before-append primitives; it does not make the worker production-ready. ## Validation - `just test -p codex-rollout` - `just test -p codex-thread-store` - `just fix -p codex-rollout` - `just fix -p codex-thread-store` - `just bazel-lock-check`
…pression-worker # Conflicts: # codex-rs/rollout/src/compression.rs # codex-rs/rollout/src/compression_tests.rs # codex-rs/rollout/src/lib.rs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69c3f6e1ac
ℹ️ 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".
2 participants
Rollout compression stack
This stack splits #24941 into reviewable steps for local rollout compression. The design is intentionally staged:
local_thread_store_compression.The key invariant is that writers append to plain
.jsonl. A.jsonl.zstfile is a cold/read representation; if a write is needed, the compressed file is materialized back to plain JSONL first. Readers prefer plain.jsonlwhen both forms exist and can fall back to the compressed sibling during transitions.The worker is deliberately the last PR and remains behind an under-development feature flag. It currently scans only
archived_sessions, not activesessions, because active sessions have the highest resume/append race risk. That means this stack does not yet compress most unarchived local history.Known race / follow-up
The remaining unresolved design question is writer/compressor coordination. Even for archived rollouts, a resume or metadata update can append while the worker is replacing the plain file with
.jsonl.zst; the current double-stat checks narrow but do not fully eliminate the window where a writer has opened the plain file before unlink. Do not treat the worker PR as production-ready until we either:The first two PRs are useful independently: they make compressed rollouts readable and make append paths safely recover back to plain JSONL. The third PR isolates the worker behavior so that coordination issue is reviewable separately.
Validation
Focused local validation for the stack includes:
just test -p codex-rolloutjust test -p codex-thread-storewhere thread-store paths were touchedjust test -p codex-featuresfor the feature flag slicejust bazel-lock-checkafter dependency graph changesjust fix -p ...passes for changed cratesCI is still the source of truth for the full platform matrix.
This PR in the stack
This is PR 3/3, based on #25088. It adds the under-development feature flag and starts the best-effort background worker when enabled. The worker currently compresses only cold archived rollouts, skips active sessions, verifies compressed output, preserves mtime and permissions, keeps a store-level lock heartbeat, and cleans stale temp files.
Stack order: