chore(worker): extract the Tokio data-plane path into tokio_conn - #355
Merged
Conversation
`main.rs` was 1509 lines spanning CLI parsing, backend construction, io_uring capability probing, ring supervision, registration, heartbeat, signal handling *and* the whole Tokio data-plane connection handler. The connection handler is the part with a natural seam. `uring_conn.rs` was already a library module, and its own docs describe it as "the completion-based counterpart to the Tokio `handle_conn` in `main.rs`" — so the two implementations of one protocol lived at different layers, one in the library and one inline in a binary. This makes them structurally parallel siblings. Moved verbatim (bodies are byte-identical; `git diff` shows moves and imports only): handle_conn, handle_control_frame, handle_put, handle_delete, sendfile_payload, read_control. Only handle_conn and read_control are `pub`; the rest stay private so the library's public API does not widen. main.rs 1509 -> 1059 lines. Measured effect worth noting: with the path in its own file, coverage now attributes it separately, and the Tokio fallback sits at 36.4% line coverage against uring_conn's 78.7%. Both paths must stay behaviourally identical, so that gap is a real gap — it was previously hidden inside main.rs's aggregate. Not addressed here; this change only makes it visible. Refs milvus-io#306.
The tokio_conn module docs linked to `sendfile_payload`, which is private, tripping `-D rustdoc::private_intra_doc_links` in the docs gate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #306. First of the splits proposed there — the one with a real seam.
Why this file, this cut
crates/talon-worker/src/main.rswas 1509 lines covering CLI parsing, backend construction, io_uring capability probing, ring supervision, coordinator registration, heartbeat, signal handling, and the entire Tokio data-plane connection handler.The connection handler is the piece that clearly does not belong there, and the codebase already says so.
uring_conn.rsis a library module whose own docs open with:So two implementations of one wire protocol lived at different layers — one in the library, one inline in a binary. They must stay behaviourally identical (same frames, same limits, same error semantics), and that is much easier to review and to keep honest when they sit side by side as siblings.
Change
Moved to a new
crates/talon-worker/src/tokio_conn.rs:handle_conn,handle_control_frame,handle_put,handle_delete,sendfile_payload,read_controlOnly
handle_connandread_controlarepub— the other four stay private so the library's public API does not widen.main.rs: 1509 → 1059 lines.The module doc explains why this path pays for
sendfile(Tokio'sTcpStreamis non-blocking, so a blockingsendfilewould spuriouslyEAGAIN, forcing aninto_std/from_stdround-trip on every transfer) and points aturing_connfor the contrast, mirroring the existing cross-reference in the other direction.This is a pure move
The function bodies are byte-identical. Verified mechanically:
differs only in the doc-comment line offset and the trailing newline. The remaining diff is imports: removed from
main.rswhat moved out, added totokio_conn.rswhat it needs, and pushed four now-test-only imports into themod testsblock.No behaviour change, so the existing suite is the regression test.
A gap this exposed
Worth flagging, because it is the kind of thing #306 argued splitting would surface. With the path in its own file, coverage attributes it separately:
The two paths are supposed to be behaviourally identical, so 36% on the fallback is a real gap — it was previously averaged into
main.rsand invisible. Not addressed in this PR; this change only makes it measurable. Worth its own issue.Verification
Not included
The other splits in #306 (
runtime.rs,coordinator/observability.rs) are separate mechanical changes and belong in separate PRs. I have deliberately lefttalon-core/src/config.rsalone — as noted in the issue, it is the source of truth for the generated configuration reference and splitting it risks theconfig-docsdrift gate for little gain.Update: PR title changed
refactor:→chore:(the repo's conventional-commits gate allowsfeat|fix|enhance|test|doc|bench|build|ci|chore, notrefactor), and fixed a self-inflicted docs failure: the module docs linked tosendfile_payload, which this PR makes private, tripping-D rustdoc::private_intra_doc_links.