Skip to content

fix(collector): finalize scheduled LOB segments off async loop - #839

Merged
proerror77 merged 1 commit into
mainfrom
codex/lob-async-segment-finalizer
Aug 11, 2026
Merged

proerror77 merged 1 commit into
mainfrom
codex/lob-async-segment-finalizer

Conversation

@proerror77

@proerror77 proerror77 commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Change contract

Move scheduled segment finalization (flush/sync/zstd/hash/manifest) onto one Tokio blocking task so the async consumer can keep processing events and publishing health. Keep at most one pending finalizer; poll completed tasks every loop without waiting, defer a due rotation while the previous task is unfinished, collect the completed prior task before the next actual rotation and before session return, and propagate task or close errors fail-closed. All post-spawn loop errors route through the common shutdown path so a pending task is never detached. Segment schema, replay/continuity rules, zstd settings, upload behavior, source-delay gate, watchdog, and runtime configuration are unchanged.

Issue relationship

Closes #838

Out of scope

No schema, threshold, upload, deployment, ECS/OSS, Gate, cutover, Live, or runtime changes.

Dependencies and merge order

None; based on exact origin/main e557059.

Focused validation

  • TDD red: base with focused tests produced the first expected missing-finalizer-helper E0425 errors.
  • Host focused tests (temporary authorized libc RUSAGE_SELF shim only; restored before commit): 4 finalizer tests passed, including completed-error polling; 15 rotation tests passed.
  • Linux target cargo check with x86_64-unknown-linux-gnu-gcc: passed.
  • Linux target scoped clippy with -D warnings: passed.
  • git diff --check: passed.

Rollout and rollback

None; code-only change. Rollback is reverting this single commit.

Scope exception

None; one source file, one behavior.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Binance LOB archiver now closes rotated segments in asynchronous blocking tasks. Session rotation and shutdown collect pending finalizers and propagate errors without blocking health or event consumption.

Changes

Asynchronous segment finalization

Layer / File(s) Summary
Finalizer task lifecycle
rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
The archiver spawns segment finalization through blocking-task helpers. finish_segment_rotation returns a pending SegmentFinalizer and propagates join errors.
Rotation and shutdown coordination
rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
run_session tracks one pending finalizer, collects it before another rotation or shutdown, and preserves existing session failures. Async tests cover blocked finalizers, error propagation, and shutdown completion.

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

Possibly related issues

  • proerror77/monday issue 552 — Concerns orphan-segment recovery and compression stalls in the same Binance LOB archiver finalization path.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation addresses issue #838 by using one blocking finalizer, collecting it at required boundaries, and preserving existing behavior.
Out of Scope Changes check ✅ Passed The one-file change is limited to asynchronous segment finalization and its tests, with no unrelated scope changes identified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly states that scheduled LOB segment finalization now runs off the async loop.
Description check ✅ Passed The description covers the change contract, issue relationship, scope, dependencies, validation, rollout, rollback, and scope exception.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/lob-async-segment-finalizer

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 2210-2218: Remove the test-only synchronous rotation helper used
by rotate_segment, including its inline closing_segment.close() lifecycle.
Update each caller to perform rotation through begin_segment_rotation,
finish_segment_rotation, and collect_segment_finalizer, preserving asynchronous
finalizer handling and existing rotation behavior.
🪄 Autofix

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: 65e833cf-c0e8-4e9a-8824-e17da8564364

📥 Commits

Reviewing files that changed from the base of the PR and between e557059 and 48f57db.

📒 Files selected for processing (1)
  • rust_hft/tools/collector/src/bin/binance-lob-archiver.rs

Comment thread rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
@proerror77
proerror77 force-pushed the codex/lob-async-segment-finalizer branch from 48f57db to 99fc1a6 Compare August 11, 2026 20:05

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48f57db3f8

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

Comment thread rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
Comment thread rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
Comment thread rust_hft/tools/collector/src/bin/binance-lob-archiver.rs
@proerror77
proerror77 force-pushed the codex/lob-async-segment-finalizer branch from 99fc1a6 to 29e6489 Compare August 11, 2026 20:09
@proerror77
proerror77 force-pushed the codex/lob-async-segment-finalizer branch from 29e6489 to 73d6998 Compare August 11, 2026 20:17
@proerror77
proerror77 merged commit b4d39f9 into main Aug 11, 2026
51 of 55 checks passed
@proerror77
proerror77 deleted the codex/lob-async-segment-finalizer branch August 11, 2026 20:28
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.

Finalize Binance LOB segments without blocking health and event consumption

1 participant