fix(export): read JSONL from one snapshot - #134
Conversation
Signed-off-by: samzong <samzong.lu@gmail.com>
|
@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: b917e56ff8
ℹ️ 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".
| options: &ExportOptions, | ||
| mut writer: W, | ||
| ) -> Result<()> { | ||
| let snapshot = store.conn.unchecked_transaction()?; |
There was a problem hiding this comment.
When recall export writes to a slow or stalled pipe while a concurrent sync keeps committing—the concurrency this change targets—this transaction remains open across every Write call. In WAL mode, that reader snapshot prevents checkpoints from advancing beyond its end mark, so sustained writes can grow the WAL until the consumer drains and may eventually exhaust disk space. Spool or materialize the export while the snapshot is open, commit it, and only then copy the completed JSONL to the caller's writer.
Useful? React with 👍 / 👎.
Signed-off-by: samzong <samzong.lu@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e58e7b2097
ℹ️ 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".
| }; | ||
|
|
||
| write_jsonl_for_sessions(store, sessions, options.includes, &mut writer) | ||
| let records = collect_session_records(store, sessions, options.includes)?; |
There was a problem hiding this comment.
With the default --limit 0, this now retains every session’s messages, usage events, and events until the entire export has been read, so an index whose contents exceed available RAM will be OOM-killed instead of streaming successfully as before. Preserve the snapshot without accumulating all ExportSessionRecords in memory—for example, serialize into a temporary spool while the snapshot is open, commit it, and then copy the spool to the requested writer.
Useful? React with 👍 / 👎.
Signed-off-by: samzong <samzong.lu@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13013ac48c
ℹ️ 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".
| let mut spool = (page_count.saturating_mul(page_size) > EXPORT_IN_MEMORY_DB_LIMIT) | ||
| .then(|| tempfile::tempfile().map(BufWriter::new)) | ||
| .transpose()?; |
There was a problem hiding this comment.
When recall session export --ids-file contains repeated IDs, a database below 8 MiB stays on this in-memory path even though the selected output is unbounded: each duplicate is loaded into a separate ExportSessionRecord, so repeating one 7 MiB session 100 times retains roughly 700 MiB before writing anything. The new page-count threshold therefore does not fully resolve the earlier memory issue; switch to the spool based on accumulated record size, or always spool selected-session exports.
Useful? React with 👍 / 👎.
Signed-off-by: samzong <samzong.lu@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
1 participant
Summary
Verification
cargo test export_jsonl_reads_every_record_from_one_snapshotcargo test export_jsonlmake checkCloses release blocker NG-07.