fix(polymarket): spill pending reference updates - #92
Conversation
Bound cold-start heap retention by staging generated updates in an unlinked private NDJSON file, then replaying them through the existing single TapeWriter transaction before state publication.
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d7179ec4b
ℹ️ 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".
| writer.write_update_results_with_sync( | ||
| PendingUpdateReader::new(self.file, self.len), | ||
| now, | ||
| sync, |
There was a problem hiding this comment.
For large cold-start batches on a constrained spool, this keeps the unlinked spill file allocated while TapeWriter appends the same updates to market-updates.ndjson, so peak disk use is roughly spill bytes plus final tape bytes. A spool with enough room for the final tape but not a second copy will now fail closed with ENOSPC and make no progress, whereas the previous in-memory batch only needed the final append space; consider freeing/truncating consumed spill data or staging the already-wrapped tape bytes in bounded chunks.
Useful? React with 👍 / 👎.
| let result = (|| -> Result<()> { | ||
| let file = self.file.as_mut().context("active tape is closed")?; | ||
| for update in updates { | ||
| let update = update?; |
There was a problem hiding this comment.
When a non-empty spill is replayed across an hour boundary, rotate(now) has already renamed the old active tape before this update? can surface a spill read/JSON error; the rollback only truncates the new active tape and does not move the rotated tape back. If the collector then exits or crashes before writing collector-state.json, the next startup recovers only from market-updates.ndjson, so any previously active tape records that existed only for crash recovery can be skipped from state recovery.
Useful? React with 👍 / 👎.
Change contract
Stage generated Polymarket reference updates in an unlinked private NDJSON spill and replay them once through the existing TapeWriter transaction, bounding cold-start heap retention without changing tape bytes or tape -> state -> health commit order.
Out of scope
Dependency / merge order
None. This PR is based directly on current
main(dc2b821f).Focused validation
cargo +1.91 test --locked -p hft-collector pending_update: 4 passed.cargo +1.91 test --locked -p hft-collector: 122 library tests passed, 1 expected ignore; 28 binance-lob-archiver tests passed; 9 materializer tests passed; raw-ops CLI test passed.cargo +1.91 clippy --locked -p hft-collector --all-targets -- -D warnings: passed.rustfmt +1.91 --edition 2021 --check tools/collector/src/polymarket_raw.rs: passed.0600permissions, and immediate unlink/no crash residue.Rollout / rollback impact
No resource or deployment setting changes. The spill is created with
create_new,0600,O_NOFOLLOW, and immediately unlinked; replay syncs and releases staging cache before a single existing TapeWriter transaction. Any spill/read/sync failure remains fail-closed and leaves state unpublished. Roll back by reverting this commit; the prior cycle-wide in-memory Vec behavior is restored.