Skip to content

fix(binance): buffer streams before initial snapshots - #127

Merged
proerror77 merged 3 commits into
mainfrom
codex/binance-startup-buffer
Jul 18, 2026
Merged

proerror77 merged 3 commits into
mainfrom
codex/binance-startup-buffer

Conversation

@proerror77

@proerror77 proerror77 commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Change contract

Start initial REST snapshot acquisition only after every required Binance WebSocket stream has connected, so startup depth events can be buffered before reconciliation.

Out of scope

  • Market-tape verifier or research materializer changes
  • OSS uploader behavior
  • Production collector cutover
  • Evaluator or strategy changes

Dependency / merge order

Depends only on merged PR #126 and current main; merge this PR before generating the Finance/Crypto research artifact.

Focused validation

  • Review budget: 1 changed file, below the repository 25-file / 750-line split threshold.
  • Red counterexample: snapshots_wait_until_every_websocket_stream_is_connected asserts zero REST requests after only one of two required streams is connected. GitHub Linux run 29635261468 failed at red commit 1cb5853e because the barrier did not exist.
  • Exact-head CI: all applicable checks passed at 075e3f5c76aa139168c02ec383aaacd0f768114a; CodeRabbit completed with no actionable findings.
  • Linux artifact: GitHub artifact 8427115579, binary SHA-256 77922320fc07aebc1af4c1d3d7f9fd830b1528b4cec9739f3bcd03ecfad02505, cloud self-test passed.
  • Pre-merge cloud runtime counterexample: isolated uploader-fenced ECS run 20260718t080031z produced 10 consecutive 60-second segments in one session (18c35374da096517-197772), BTCUSDT/SOLUSDT both bridged, every data/manifest SHA verified, zero sequence gaps, zero internal reconnects, zero process restarts, and production PIDs/restart counters/invocation IDs unchanged.
  • Frozen gate evidence SHA-256: 1bf1253170bb6fc3597a0a5ab71cfa42dfa653e80e77687a30bb01a85094f45f.

Rollout / rollback impact

Candidate-only validation completed; no production service or symlink changed. Rollback is the previous immutable collector binary; no state migration is required. Production cutover remains a separate gate and is not part of this PR.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Websocket session startup now waits for all expected streams to connect before issuing REST snapshot requests. Connection acknowledgments feed a readiness gate, which then runs the existing snapshot pipeline. Session metadata uses the computed stream count, and an async test verifies ordering and shutdown.

Changes

Snapshot startup coordination

Layer / File(s) Summary
Connection acknowledgment wiring
rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
run_session computes the expected stream count, passes an acknowledgment channel to websocket tasks, and records that count in session metadata. receive_url signals after connecting.
Snapshot readiness gate and validation
rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
Snapshot production waits for every expected connection signal before emitting snapshots and InitialSnapshotsComplete; the async test verifies request ordering, output, and shutdown.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Session
  participant WebsocketStreams
  participant ReadinessGate
  participant SnapshotProducer
  Session->>WebsocketStreams: Start expected websocket tasks
  WebsocketStreams->>ReadinessGate: Send connection acknowledgments
  ReadinessGate->>SnapshotProducer: Start after all acknowledgments
  SnapshotProducer-->>Session: Emit snapshots and InitialSnapshotsComplete
Loading

Possibly related PRs

  • proerror77/monday#126: Both changes modify Binance session orchestration and snapshot production in binance-lob-archiver.rs.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main Binance startup sequencing change.
Description check ✅ Passed The description covers the required template sections with clear change, validation, rollout, and scope notes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/binance-startup-buffer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proerror77
proerror77 marked this pull request as ready for review July 18, 2026 07:30
@proerror77

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@proerror77
proerror77 merged commit 093edb1 into main Jul 18, 2026
19 checks passed
@proerror77
proerror77 deleted the codex/binance-startup-buffer branch July 19, 2026 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant