Skip to content

refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait - #12447

Merged
tanmayv25 merged 2 commits into
mainfrom
refactor/router-streaming-dispatch-seam
Aug 4, 2026
Merged

refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait#12447
tanmayv25 merged 2 commits into
mainfrom
refactor/router-streaming-dispatch-seam

Conversation

@tanmayv25

@tanmayv25 tanmayv25 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Extract the final-hop transport in PushRouter behind a new
StreamingDispatch<T, U> trait. After PushRouter selects a worker, the
final hop goes through this trait object instead of a fixed
AddressedPushRouter. PushRouter keeps instance selection, occupancy, fault
detection, and migration; only the transport below the seam can now be swapped.
AddressedPushRouter (the request plane) is the default implementation and its
behavior is unchanged.

This is a behavior-preserving refactor with no new functionality — it makes
the final-hop transport pluggable behind a single typed seam. No alternate
transport implementation is included here.

Changes

  • New StreamingDispatch<T, U> trait (generate, generate_bidirectional,
    on_instance_removed / on_instance_added), with AddressedPushRouter as the
    default impl delegating to its existing AsyncEngine path.
  • PushRouter.addressed: Arc<AddressedPushRouter> -> Arc<dyn StreamingDispatch<T, U>>.
  • spawn_instance_removal_watcher is generic and drives discovery-removal
    cleanup through the trait.
  • New PushRouter::from_client_with_dispatch to inject a custom dispatch
    (unused in this PR).
  • Rename inherent AddressedPushRouter::generate_bidirectional ->
    dispatch_bidirectional (disambiguates from the trait method); make
    AddressedRequest::into_parts public.

Validation

  • cargo build -p dynamo-runtime and -p dynamo-llm (downstream) compile.
  • cargo test -p dynamo-runtime egress suite: 67 passed, 0 failed — the
    request-plane path is exercised unchanged.

Enables pluggable final-hop transports behind the router seam for future work.

@datadog-official

datadog-official Bot commented Jul 30, 2026

Copy link
Copy Markdown

Pipelines

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 43.93% (-5.92%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 87a8ef8 | Docs | Datadog PR Page | Give us feedback!

@tanmayv25 tanmayv25 changed the title refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait refactor(runtime): extract PushRouter transport behind StreamingDispatch trait Jul 30, 2026
@tanmayv25
tanmayv25 marked this pull request as ready for review July 30, 2026 23:38
@tanmayv25
tanmayv25 requested a review from a team as a code owner July 30, 2026 23:38
@tanmayv25
tanmayv25 marked this pull request as draft July 30, 2026 23:39

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread lib/runtime/src/pipeline/network/egress/push_router.rs
Comment thread lib/runtime/src/pipeline/network/egress/push_router.rs
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Streaming dispatch integration

Layer / File(s) Summary
Streaming dispatch contract
lib/runtime/src/pipeline.rs, lib/runtime/src/pipeline/network/egress/addressed_router.rs
Exports StreamingDispatch, makes AddressedRequest::into_parts public, renames bidirectional dispatch, and defines the dispatch lifecycle interface.
Addressed router implementation
lib/runtime/src/pipeline/network/egress/addressed_router.rs
Implements unary and bidirectional dispatch plus instance removal and addition handling for AddressedPushRouter.
PushRouter dispatch wiring
lib/runtime/src/pipeline/network/egress/push_router.rs
Uses type-erased dispatch implementations, routes discovery callbacks through them, and adds construction with a caller-supplied dispatcher.

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

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the refactor and validation, but it omits the required Related Issues section and reviewer starting point. Add the required Related Issues section with either an issue reference or a confirmed no-issue checkbox, and identify where reviewers should start.
✅ Passed checks (4 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 and concisely describes the extraction of the PushRouter transport behind the StreamingDispatch trait.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 `@lib/runtime/src/pipeline/network/egress/push_router.rs`:
- Around line 326-333: The watcher deduplication in
spawn_instance_removal_watcher must account for dispatch identity, not only
EndpointId. Enforce a single StreamingDispatch per endpoint or update
ENDPOINT_WATCHER_ACTIVE and its ownership checks to distinguish dispatch
instances, ensuring every independent dispatch receives on_instance_removed and
on_instance_added callbacks.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8fb16d73-7b31-48de-a591-56e887a90d69

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc7c4e and d21cfc7.

📒 Files selected for processing (3)
  • lib/runtime/src/pipeline.rs
  • lib/runtime/src/pipeline/network/egress/addressed_router.rs
  • lib/runtime/src/pipeline/network/egress/push_router.rs

Comment thread lib/runtime/src/pipeline/network/egress/push_router.rs
@tanmayv25
tanmayv25 force-pushed the refactor/router-streaming-dispatch-seam branch from d21cfc7 to d5c3bb0 Compare July 31, 2026 22:11
@tanmayv25 tanmayv25 changed the title refactor(runtime): extract PushRouter transport behind StreamingDispatch trait refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait Jul 31, 2026
@tanmayv25
tanmayv25 force-pushed the refactor/router-streaming-dispatch-seam branch 2 times, most recently from f9851ad to 1d99755 Compare July 31, 2026 22:25
@tanmayv25
tanmayv25 marked this pull request as ready for review August 1, 2026 00:11
…Dispatch trait

## Summary

Extract the final-hop transport in `PushRouter` behind a new
`StreamingDispatch<T, U>` trait. `PushRouter` keeps instance selection,
occupancy, fault detection, and migration; its `addressed` field becomes a
trait object so the transport below the seam can be swapped.
`AddressedPushRouter` (the request plane) is the default implementation and its
behavior is unchanged.

This is a behavior-preserving refactor with no new functionality. It makes the
final-hop transport pluggable behind a single typed seam; no alternate transport
is included in this change.

Changes:
- New `StreamingDispatch<T, U>` trait (`generate`, `generate_bidirectional`,
  `on_instance_removed` / `on_instance_added`), with `AddressedPushRouter` as the
  default impl delegating to its existing `AsyncEngine` path.
- `PushRouter.addressed`: `Arc<AddressedPushRouter>` -> `Arc<dyn StreamingDispatch<T, U>>`.
- `spawn_instance_removal_watcher` is generic and drives discovery-removal cleanup
  through the trait.
- New `PushRouter::from_client_with_dispatch` to inject a custom dispatch.
- Rename inherent `AddressedPushRouter::generate_bidirectional` ->
  `dispatch_bidirectional` (disambiguates from the trait method); make
  `AddressedRequest::into_parts` public.

## Validation

- `cargo build -p dynamo-runtime` and `-p dynamo-llm` (downstream) compile.
- `cargo test -p dynamo-runtime` egress suite: 67 passed, 0 failed — the
  request-plane path is exercised unchanged.

Signed-off-by: tanmayv25 <tanmay2592@gmail.com>
@tanmayv25
tanmayv25 force-pushed the refactor/router-streaming-dispatch-seam branch from 1d99755 to 7c412cf Compare August 1, 2026 00:20

@PeaBrane PeaBrane left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ AI-assisted review complete. Approving with two non-blocking comments for human evaluation.

Comment thread lib/runtime/src/pipeline/network/egress/push_router.rs
Comment thread lib/runtime/src/pipeline/network/egress/push_router.rs
Construct a PushRouter through from_client_with_dispatch and assert a
caller-supplied dispatch (not the default AddressedPushRouter) receives
unary and bidirectional requests with the selected address/instance, and
that discovery removal/re-addition reach its on_instance_removed /
on_instance_added hooks. Existing tests only exercised the default adapter.

Signed-off-by: tanmayv25 <tanmay2592@gmail.com>
@tanmayv25
tanmayv25 enabled auto-merge (squash) August 3, 2026 23:34
@tanmayv25
tanmayv25 merged commit 32b227b into main Aug 4, 2026
178 of 180 checks passed
@tanmayv25
tanmayv25 deleted the refactor/router-streaming-dispatch-seam branch August 4, 2026 00:08
hhzhang16 added a commit that referenced this pull request Aug 4, 2026
dyn-3691-extract-shared-target-pid-cuda-customstorage-operation-layer

* 'main' of https://github.com/ai-dynamo/dynamo: (50 commits)
  docs(cli): correct removed vLLM prefill-worker flag reference (#12581)
  docs(operator): reserve webhook Ignore for emergencies (#12563)
  ci(docs): make previews and checks match what actually publishes (#12339)
  refactor(vllm): organize custom encoder modules (#12416)
  feat(llm): Select reasoning output field via env var (#11464)
  feat(runtime): add TLS support to TCP request plane (#10921)
  fix: convert conditional disagg sglang warning to httperror 400 (#12578)
  feat(operator): add runtime feature gates (#12421)
  refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait (#12447)
  feat(replay): add deterministic canonical offline reports (#12363)
  build: bump ModelExpress to 0.5.0(OPS-7978) (#12455)
  fix(mocker): use logical KV tokens for decode timing (#12583)
  fix(examples): update Triton example for CUDA 13 + fix libdcgm copy (DYN-3697) (#12577)
  refactor(operator): implement composition-first DGD reconciliation (#12283)
  feat(frontend): add basetenkenizer backend (#12376)
  fix(profiler): configure rapid mocker without planner (#12573)
  docs(vllm): correct worker-role flags and document --kv-transfer-config (#12568)
  ci: add Kubernetes deploy test to nightly (#12090)
  fix(container): reuse pinned protoc in runtime image (#12535)
  feat(self-host): flip DYN_SELF_HOST_METADATA default to ON (gh-8749) (#11417)
  ...

Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants