Skip to content

feat(pool-relay): register the routing pool with Global Discovery - #18077

Merged
jioffe502 merged 4 commits into
mainfrom
jioffe/llm-214-pool-registration
Nov 4, 2026
Merged

jioffe502 merged 4 commits into
mainfrom
jioffe/llm-214-pool-registration

Conversation

@jioffe502

@jioffe502 jioffe502 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Global Discovery lists a pool in GET /pools only while the pool has a registration inside its lease. That holds in every mode: registrations, karmada, and mcs. Until now nothing sent that registration. A Relay started with --routing-pool published its state to the Global Routers, but its pool never appeared in GET /pools. With this change the Relay registers the routing pool with POST /pools and renews the registration on the lease heartbeat. When the Relay stops, it stops renewing, and the pool leaves GET /pools one lease later.

Linear: LLM-214

image image

Key Changes

  • PoolHeartbeat in dynamo-global-discovery-client (heartbeat.rs) registers a pool and renews it on each lease's heartbeat interval. It retries a failed call after 1 s, doubles the wait up to the heartbeat interval, and makes one more call 1 s before the lease ends, so an outage that ends before then keeps the pool listed. It retries every failure, 501 included. It reads the registration from a watch of Option<PoolRegistration> and sends nothing while the value is None.
  • RoutingPoolRegistration (lib/llm/src/pool_relay/routing_pool.rs) builds the registration from the Relay's serving topology. A registration names one model:
    • It keeps the registered model while the model is in the topology, in any state. Each replica reports its own readiness, so following readiness could split active-active Relays between two models.
    • Otherwise it picks the one Ready model, or the only model while none is Ready.
    • With no model, or with several to choose from, it registers nothing and logs why.
  • Adds --routing-pool-region and --routing-pool-frontends to the Python CLI, the bindings, and PoolRelayConfig. --routing-pool now requires --routing-pool-region. PoolRelay::start rejects a frontend that is not an HTTP or HTTPS URL.
  • The new health field routing_pool_unregistered_reason says why the pool is not registered: no model yet, several models, a failed call, or the Relay stopped. It does not affect healthy.

Validation

  • Unit tests:
    • The PoolHeartbeat tests run on paused Tokio time. They check renewal timing, retries and the backoff reset, the retry before the lease ends, None handling, and stopping on cancel or drop.
    • The RoutingPoolRegistration tests check model selection, staying on the registered model, withdrawing the registration when the Relay stops, and the health reason.
    • The CLI tests check the new flags.
    • 21 passed in dynamo-global-discovery-client, 269 in dynamo-llm pool_relay::, and 96 in pool_relay/tests/test_cli.py.
  • cargo clippy --no-deps --all-targets -- -D warnings is clean on dynamo-global-discovery-client, dynamo-llm, and the Python bindings.
  • End to end on one host. Global Discovery ran in registrations mode with a 2 s heartbeat and a 6 s lease, and a dynamo.mocker worker shared file discovery with the Relay.
    • GET /pools listed the pool with the topology's model, Qwen/Qwen3-0.6B, and with the region and frontends.
    • The pool stayed listed for 18 s, three leases.
    • It left GET /pools 5.0 s after the Relay got SIGTERM, and 5.4 s after it in an earlier run.
  • Neither mcs nor karmada mode ran end to end, because both need Kubernetes. The Global Discovery tests in backends/registered.rs cover listing a registered pool in those modes.

Issues and Ongoing Work

  • Requiring --routing-pool-region breaks no deployment: --routing-pool merged in feat(pool-relay): share live pool state across router connection modes #17716 on 2026-11-04, and nothing deploys it yet.
  • The operator does not set the new flags yet. Setting them, and adding the DynamoPoolRelay CRD fields, is follow-up work.
  • The registration leaves capacity and capabilities empty. The Global Router design's RegisterPool() carries both, and blue/green relies on capacity changes, but nothing reads either field yet, and the Relay has no GPU SKU source. Filling them is follow-up work.
  • Two questions are open with Hannah. Can a DGD serve more than one model? Does DYN_CLUSTER_ID match the cluster name MCS uses?
  • Global Routers route the pool only by its registered model, not by aliases or LoRA adapters.
  • A change from model A to model B reaches Global Discovery at the next heartbeat, up to 5 s later.
Pull Request Checklist
  • I used Dynamo's /pr-checklist skill.

Devin Review

Summary by CodeRabbit

  • New Features
    • Pool Relay can register with Global Discovery using a configured region and optional frontend URLs, and renew its registration lease.
    • Registration follows the serving topology’s selected model. Relay health reports why registration is unavailable or failing.
  • Bug Fixes
    • Improved error details include the full chain of underlying causes.
  • Documentation
    • Updated configuration and setup guidance with routing-pool requirements, registration behavior, and health status details.

PoolHeartbeat registers a pool with POST /pools and renews it on each
lease's heartbeat interval, sending the latest registration each time.
A failed call is retried after 1 s, doubling up to the heartbeat
interval, and once more 1 s before the lease ends, so an outage that
ends in time does not lapse it. Every failure is retried, a 501
included: every Global Discovery mode takes registrations, and with
MCS a pool is listed only while its registration is within the lease.
Cancel and drop stop it. The service has no DELETE, so a stopped pool
drops out one lease later.

error_chain moves to client.rs, next to DiscoveryClientError, now that
the caches and the heartbeat share it.

Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
PoolHeartbeat now takes a watch of Option<PoolRegistration>. While the
value is None it sends nothing, so the service drops the pool once its
lease passes, and the next registration goes out at once. The PoolRelay
needs this because a registration names a model, and the Relay learns
the model only when workers appear.

Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Global Discovery lists a pool only while it has a registration inside
its lease, in every mode. With --routing-pool, the Relay now registers
the pool with POST /pools and renews it on the lease heartbeat through
PoolHeartbeat.

RoutingPoolRegistration builds the registration from the serving
topology. It keeps the registered model while the model is in the
topology, and otherwise picks the one Ready model, or the only model
while none is Ready. The new flags --routing-pool-region (required with
--routing-pool) and --routing-pool-frontends supply the location and
the frontends. The health field routing_pool_unregistered_reason says
why the pool is not registered.

Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
@github-actions github-actions Bot added feat documentation Improvements or additions to documentation labels Nov 4, 2026
@jioffe502
jioffe502 marked this pull request as ready for review November 4, 2026 16:34
@jioffe502
jioffe502 requested review from a team as code owners November 4, 2026 16:34

@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 4 potential issues.

Devin Review

Comment thread lib/llm/src/pool_relay/routing_pool.rs Resolved
Comment thread lib/llm/src/pool_relay/routing_pool.rs Resolved
Comment thread lib/llm/src/pool_relay/routing_pool.rs Resolved
Comment thread components/src/dynamo/pool_relay/cli.py Resolved
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The Pool Relay accepts routing-pool region and frontend settings. It selects a model from the serving topology and registers the pool with Global Discovery. A lease heartbeat renews the registration, and health reports the reason when registration is absent.

Changes

Routing-pool registration

Layer / File(s) Summary
Routing-pool configuration and validation
components/src/dynamo/pool_relay/*, components/src/dynamo/pool_relay/tests/test_cli.py, lib/bindings/python/rust/llm/pool_relay.rs, lib/bindings/python/src/dynamo/_core.pyi, lib/llm/src/pool_relay/host.rs, docs/fern/pages/reference/components/pool-relay-configuration.md
The CLI and Python constructor accept region and frontend settings. Configuration parsing and Pool Relay startup validate those settings and require a region when a routing pool is configured.
Global Discovery lease heartbeat
lib/global-discovery-client/src/heartbeat.rs, lib/global-discovery-client/src/client.rs, lib/global-discovery-client/src/cache.rs, lib/global-discovery-client/src/lib.rs
The client adds PoolHeartbeat and RegistrationStatus. The heartbeat submits registrations, renews leases, retries failed requests, and retains status. Error-chain formatting is shared by the client and cache.
Topology-based pool registration
lib/llm/src/pool_relay.rs, lib/llm/src/pool_relay/routing_pool.rs, lib/llm/src/pool_relay/host.rs, lib/llm/src/pool_relay/docs/architecture.md, docs/fern/pages/reference/components/pool-relay-configuration.md, lib/global-frontend/README.md
The Relay selects and registers a model based on topology updates. It reports reasons for withheld or failed registration. The documentation describes registration, renewal, model selection, and the health field.

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

Merge Risk: Minimal · up to bd0f4

No identified issue prevents merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title "feat(pool-relay): register the routing pool with Global Discovery" matches the main change. The changeset adds PoolHeartbeat, which registers the routing pool with Global Discovery via POST…
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.
Description check Passed The description includes all required sections: Overview, Key Changes, Validation, Issues and Ongoing Work, and the pull request checklist. It provides detailed scope, testing results, known limitatio…
Full details: Title check

Explanation

The title "feat(pool-relay): register the routing pool with Global Discovery" matches the main change. The changeset adds PoolHeartbeat, which registers the routing pool with Global Discovery via POST /pools and renews it on the lease heartbeat, and RoutingPoolRegistration, which selects the model to register. The title is specific and concise. It follows a conventional-commit style and does not list files.

Full details: Description check

Explanation

The description includes all required sections: Overview, Key Changes, Validation, Issues and Ongoing Work, and the pull request checklist. It provides detailed scope, testing results, known limitations, and follow-up work. It references Linear issue LLM-214 instead of the template's Closes #XXXX format, but the template states that issue linking is advisory.

Full details: Docstring Coverage

Explanation

Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 12 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8b4ad6a and bd0f4d0.

Files selected for processing: 15
  • components/src/dynamo/pool_relay/__main__.py
  • components/src/dynamo/pool_relay/cli.py
  • components/src/dynamo/pool_relay/tests/test_cli.py
  • docs/fern/pages/reference/components/pool-relay-configuration.md
  • lib/bindings/python/rust/llm/pool_relay.rs
  • lib/bindings/python/src/dynamo/_core.pyi
  • lib/global-discovery-client/src/cache.rs
  • lib/global-discovery-client/src/client.rs
  • lib/global-discovery-client/src/heartbeat.rs
  • lib/global-discovery-client/src/lib.rs
  • lib/global-frontend/README.md
  • lib/llm/src/pool_relay.rs
  • lib/llm/src/pool_relay/docs/architecture.md
  • lib/llm/src/pool_relay/host.rs
  • lib/llm/src/pool_relay/routing_pool.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@jioffe502
jioffe502 enabled auto-merge (squash) November 4, 2026 20:51
@devin-ai-integration

Copy link
Copy Markdown
Contributor

✅ Dynamo PR CI passed — run 41868450128 (attempt 1) on bd0f4d0a50

Gate checks: ✅ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Posted automatically by Devin for run 41868450128. Updated on every full-CI run of this PR.

@jioffe502
jioffe502 merged commit e51394e into main Nov 4, 2026
159 checks passed
@jioffe502
jioffe502 deleted the jioffe/llm-214-pool-registration branch November 4, 2026 21:50
ishandhanani added a commit to ishandhanani/dynamo that referenced this pull request Nov 5, 2026
Second upstream merge for the split v2 PR: 36 commits past the first
merge's base 182039d, 17 conflicted paths, each upstream change placed
in the crate that owns the code at the stack head, upstream behavior whole,
the stack's moves intact, no shims.

- ai-dynamo#17723 flatten policy classes: flat PolicyProfile (from_classes,
  UnknownPolicyClassError) in lib/router/admission profile.rs; the YAML
  half (default_policy_class, removed keys rejected) in lib/router/kv-router
  policy_config.rs; KvSchedulerError::UnknownPolicyClass via From in the
  kernel; 400/InvalidArgument in lib/llm (UnknownPolicyClassRefusal) and
  ext-proc.
- ai-dynamo#17724 EDF ordering: RouterQueuePolicy::Edf in admission profile.rs;
  due-time order, DueKey and class budgets in admission queue.rs; the
  restored score keys (RouterSchedulingPolicy::score_policy) as the
  kernel's policy.rs (ordered-float back in dynamo-scheduler).
- ai-dynamo#17725 controlled delay: head expiry and with_class_budget_expiry in
  admission queue.rs; ClassTimers, due-time checks, DRR charged on
  admission (extract_next/settle) and the retained overlap refresh in the
  kernel's queue.rs and overlap_refresh.rs; metrics in lib/llm.
- ai-dynamo#17437 image-aware KV routing for TRT-LLM sidecars: zmq_wire/trtllm.rs
  in lib/kv-events; the sidecar depends on kv-events, kv-hashing,
  multimodal and protocol instead of kv-router.
- ai-dynamo#14775 prefill cancellation, ai-dynamo#17716/ai-dynamo#18077 pool relay, ai-dynamo#17707 NVTX
  range, ai-dynamo#18102 LiveEngine buffering, ai-dynamo#17887 shared replay clock: lib/llm,
  bindings, mocker, bench, auto-merged with import re-points.

Tests: upstream's queue tests split as the stack does (YAML-driven policy
queue tests in kv-router through testing hooks; kernel tests build the
flat profile through PolicyProfile::from_classes); every upstream test in
the touched modules is present (97 kernel queue, 40 policy queue, 21
policy config, 53 config, 15 policy, 52 zmq_wire). AGENTS.md text for the
queue placed in the two owner crates. CODEOWNERS regenerated; four
lockfiles regenerated and --locked clean.

Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation feat size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants