fix(collector): fence segment rotation

[proerror77]

Change contract

Atomically fence every Binance collector segment rotation so every producer drains events captured before the rotation boundary, preserves original received timestamps, and rejects residual pre-boundary events. Preserve snapshot retry delay and budget across rotation pauses, and fail closed immediately if a producer exits while barriers are pending.

Out of scope

Research, snapshot/evaluator behavior, historical backfill, funding/OI/reference collectors, and any production cutover before a fresh full shadow gate succeeds.

Dependency or merge order

None. Base: main. This independent runtime-safety fix follows the already-merged #224 code contract and must merge before a new shadow candidate is built.

Focused validation

• cargo check -p hft-collector --locked
• cargo clippy -p hft-collector --all-targets --features collector-binance --no-deps --locked -- -D warnings
• cargo test -p hft-collector --features collector-binance --locked (217 library tests plus 63 archiver tests passed; 3 network/subprocess tests ignored)
• bash deployment/aliyun/test-rust-lob-control-plane.sh
• git diff --check
• Direct regressions cover reconnect-proof ordering, bounded timeout ACK, rotation-close teardown, retry-backoff pause preservation, and producer exit during a rotation barrier.

Rollout / rollback impact

Deploy only as an isolated shadow collector under the single ECS controller. Promote only after a fresh >=3600-second gate proves positive agg_trade, zero LOB sequence gaps, manifest/SHA/_SUCCESS readback, and replay-safe ordering. Roll back by keeping or restoring the existing production collector binary; no data is rewritten.

Atomic scope exception

This two-file, 1,835 non-generated-line diff exceeds the normal review-size threshold but cannot safely split: producer barriers, pause-safe waits, retry preservation, bounded proof handling, resume-before-compression, failure teardown, and the fail-closed segment guard form one boundary-safety and rollback unit. Named reviewers /root/collector_boundary_spec_review and /root/collector_boundary_standards_review accepted the atomic boundary exception; the latest scoped review fixes are covered by targeted counterexamples and require fresh CI before merge.

PRD

Not required: this is a narrowly specified regression fix for the existing collector behavior contract.

Refs #224

[coderabbitai]

📝 Walkthrough

Walkthrough

The Binance LOB archiver now coordinates websocket and snapshot producers through epoch-based rotation barriers, drains events before segment boundaries, refactors segment closing, and rejects events timestamped before a segment’s start.

Changes

Rotation and segment integrity

Layer / File(s)	Summary
Rotation contract and controller flow rust_hft/tools/collector/src/bin/binance-lob-archiver.rs	Adds rotation epochs, producer barrier acknowledgements, coordination channels, barrier-aware event draining, and explicit archive-only handling.
Producer pause and resume handling rust_hft/tools/collector/src/bin/binance-lob-archiver.rs	Updates websocket reconnect/proof flows and snapshot timing to pause, acknowledge rotation epochs, and resume after controller signals.
Segment lifecycle and boundary validation rust_hft/tools/collector/src/bin/binance-lob-archiver.rs, rust_hft/tools/collector/src/lob_archiver.rs	Separates segment preparation from closing and rotation, and rejects writes whose received_at_ns predates the segment start.
Rotation and snapshot synchronization tests rust_hft/tools/collector/src/bin/binance-lob-archiver.rs, rust_hft/tools/collector/src/lob_archiver.rs	Tests producer barriers, reconnect and snapshot pausing, event draining before boundaries, and invalid pre-boundary timestamps.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant run_session
  participant WebsocketProducer
  participant SnapshotProducer
  participant EventQueue
  participant Segment
  run_session->>WebsocketProducer: publish rotation epoch
  run_session->>SnapshotProducer: publish rotation epoch
  WebsocketProducer->>EventQueue: emit RotationBarrier
  SnapshotProducer->>EventQueue: emit RotationBarrier
  run_session->>EventQueue: drain captured events
  run_session->>Segment: close and begin next segment
  run_session->>WebsocketProducer: publish resume epoch
  run_session->>SnapshotProducer: publish resume epoch

Loading

Possibly related PRs

• proerror77/monday#124: Related segment rotation and replay-safe checkpoint handling.
• proerror77/monday#127: Related snapshot producer and websocket readiness flow.
• proerror77/monday#346: Related draining and quiescing of events around segment boundaries.

🚥 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 change: fencing collector segment rotation.
Description check	✅ Passed	The description covers the required sections and includes contract, scope, dependencies, validation, rollout, and scope-exception details.

✨ 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/collector-unsafe-overlap

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust_hft/tools/collector/src/bin/binance-lob-archiver.rs`:
- Around line 973-984: Update the rotation handling in the session loop around
begin_segment_rotation so failures from rotation or closing the outgoing segment
are captured in failure and exit through the loop’s existing break-based
teardown path instead of propagating with ?. Keep the closing and next segment
values scoped as needed, preserve assignment of the next segment on success, and
ensure shutdown, draining, replay-safety marking, segment closure, and health
reporting still execute.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a99a838f-3a4d-4954-8f1d-0989047afd73

📥 Commits

Reviewing files that changed from the base of the PR and between 41d08e0 and f07e06f.

📒 Files selected for processing (2)

• rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
• rust_hft/tools/collector/src/lob_archiver.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f07e06fc2d

ℹ️ 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".