Read compressed rollouts and materialize before append#25087
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6f6a17e8a
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ 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". |
|
Clippy failure is due to |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 005c818463
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ 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". |
Why
Local rollout compression needs a cold
.jsonl.zstrepresentation 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
.jsonl.zstrollouts..jsonlas the logical/stored rollout path while allowing read paths to open either plain or compressed storage..jsonlbefore append-mode writes, including resume and direct metadata append paths.Remaining Stack
#25089 remains the separate worker PR. It is based directly on this PR and stays behind the disabled
local_thread_store_compressionfeature 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-rolloutjust test -p codex-thread-storejust fix -p codex-rolloutjust fix -p codex-thread-storejust bazel-lock-check