Skip to content

revert(router): keep session affinity local [DYN-3249] - #11217

Merged
PeaBrane merged 3 commits into
mainfrom
rupei/revert-distributed-session-affinity
Jul 6, 2026
Merged

revert(router): keep session affinity local [DYN-3249]#11217
PeaBrane merged 3 commits into
mainfrom
rupei/revert-distributed-session-affinity

Conversation

@PeaBrane

@PeaBrane PeaBrane commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove the Runtime/discovery distributed-claim surface and restore process-local session affinity semantics.
  • Retain crate-private affinity lifecycle types, complete AffinityTarget propagation, and the explicit prefill DP-rank path.
  • Replace the cross-replica e2e with one-frontend coverage for both etcd and FileStore, and document raw ingress-header hashing for multi-frontend deployments.

Validation

  • cargo test -p dynamo-llm --no-default-features session_affinity (28 passed)
  • .venv/bin/python -m pytest tests/router/test_router_e2e_with_mockers.py::test_mocker_session_affinity -q (2 passed)
  • /clean for lib/runtime and lib/llm (cargo clippy --no-default-features -- -D warnings and cargo fmt)
  • cargo metadata --locked --no-deps for the workspace and standalone Python/KVBM/runtime-example lockfiles

Open in Devin Review

Summary by CodeRabbit

  • Bug Fixes

    • Session IDs now behave more predictably for routing: requests stay pinned to the selected worker, and mismatched or unavailable targets are handled more safely.
    • Session-affinity state is cleaned up more reliably after errors, cancellations, and stream completion.
    • Routing now better preserves worker and rank choices when an explicit target is partially specified.
  • Documentation

    • Clarified session-ID and session-affinity behavior across router and frontend docs, including how routing, tracing, and TTL settings interact.

Signed-off-by: PeaBrane <yanrpei@gmail.com>
@PeaBrane
PeaBrane requested review from a team as code owners July 3, 2026 20:34
@PeaBrane
PeaBrane requested a review from a team July 3, 2026 20:34
@github-actions github-actions Bot added revert documentation Improvements or additions to documentation router Relates to routing, KV-aware routing, etc. labels Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@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.

Open in Devin Review

Comment thread lib/llm/src/session_affinity/coordinator.rs
Comment thread lib/runtime/src/storage/kv/file.rs Outdated
Comment thread lib/runtime/src/storage/kv/file.rs Outdated
Comment thread lib/llm/src/kv_router/push_router.rs
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR replaces the distributed, claim-based session affinity model with a simplified router-local binding model. The AffinityCoordinator and related routers (KvPushRouter, SessionAffinityPushRouter) now use AffinityAcquire/AffinityInitialization/AffinityLease keyed by session ID instead of distributed claims. Discovery backends (Kubernetes, KV store) drop claim-related types and methods entirely. Documentation, CLI help text, and tests are updated to reflect the new semantics. Two unrelated fixes adjust the notify crate's macOS feature and file-watch path canonicalization.

Changes

Session affinity model simplification

Layer / File(s) Summary
Affinity coordinator core rewrite
lib/llm/src/session_affinity/coordinator.rs
Coordinator storage keys switch from claim keys to session IDs; AffinityAcquire/AffinityInitialization/AffinityLease lifecycle with validate_bound_target, direct entry-count updates, and simplified test notify hooks replace the prior distributed/probe-based machinery.
KV push router affinity acquisition and dispatch
lib/llm/src/kv_router/push_router.rs, lib/llm/src/kv_router/push_router/request_guard.rs, lib/llm/src/kv_router/push_router/selection.rs
select_with_affinity returns AffinityAcquire; dispatch paths invalidate on failure and wrap streams via operation.into_stream; a new merge_affinity_pin helper reconciles explicit and affinity pins; tests updated.
SessionAffinityPushRouter direct/query/generate paths
lib/llm/src/session_affinity/push_router.rs
New direct_target and acquire_routable helpers replace resolve_affinity/book_and_dispatch_exact_target; dp_rank is propagated into dispatch and recorded targets; tests rewritten for rollback and invalidation behavior.
Session affinity test suite migration
lib/llm/src/session_affinity/tests.rs
Tests migrated from resolve_local flows to Initialize → commit → Bound acquisition, with new coverage for contract validation and invalidation.
Discovery claim API removal
lib/runtime/src/discovery/mod.rs, lib/runtime/src/discovery/kube.rs, lib/runtime/src/discovery/kv_store.rs, lib/runtime/src/pipeline/network/egress/push_router.rs
Claim-related public types, trait methods, struct fields, and book_and_dispatch_exact/peek_worker_for_request helpers are removed.
E2E session affinity test updates
tests/router/common.py, tests/router/test_router_e2e_with_mockers.py
Two-frontend distributed session-affinity e2e test replaced with a single-frontend session-pinning test.
Session affinity documentation updates
components/src/dynamo/common/configuration/groups/router_args.py, docs/agents/session-ids.md, docs/components/frontend/nvext.md, docs/components/router/router-configuration.md
CLI help text and docs updated to describe router-local binding semantics instead of distributed claim coordination.

Unrelated storage/runtime fixes

Layer / File(s) Summary
KV storage and file-watch adjustments
lib/runtime/src/storage/kv.rs, lib/runtime/Cargo.toml, lib/runtime/src/storage/kv/file.rs
Removes unused Manager::is_memory, switches macOS notify feature to macos_fsevent, and changes path canonicalization/delete-event filtering in the file watcher.

Estimated code review effort: 5 (Critical) | ~120 minutes

Related PRs: None specified.

Suggested labels: area/router, area/session-affinity, breaking-change, needs-rust-review

Suggested reviewers: Reviewers familiar with the kv_router and session_affinity crates in lib/llm and lib/runtime discovery modules.

🐰 A claim once bound the session tight,
Now local leases hold the light,
No distant etcd, no shared claim,
Just router memory, ever the same,
Hop, commit, dispatch — done right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is missing the required template sections for Overview, Details, reviewer start, and Related Issues. Add the required template sections, especially Overview, Details, Where should reviewer start?, and a completed Related Issues section with an issue link or confirmed none.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: restoring local session affinity.
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.

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/storage/kv/file.rs`:
- Line 493: The delete handling in the event match is too narrow because
`notify` on macOS can report removals as `RemoveKind::Any`, so
`WatchEvent::Delete` may be missed. Update the match arm in the `file.rs` event
processing logic around `EventKind::Remove(...)` to treat both
`RemoveKind::File` and `RemoveKind::Any` as delete events, keeping the existing
watch-stream behavior consistent.
🪄 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: 9ac19023-fa20-45d1-abf6-b15a6071ba51

📥 Commits

Reviewing files that changed from the base of the PR and between 4808d9a and 9060b03.

⛔ Files ignored due to path filters (4)
  • Cargo.lock is excluded by !**/*.lock
  • lib/bindings/kvbm/Cargo.lock is excluded by !**/*.lock
  • lib/bindings/python/Cargo.lock is excluded by !**/*.lock
  • lib/runtime/examples/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (20)
  • components/src/dynamo/common/configuration/groups/router_args.py
  • docs/agents/session-ids.md
  • docs/components/frontend/nvext.md
  • docs/components/router/router-configuration.md
  • lib/llm/src/kv_router/push_router.rs
  • lib/llm/src/kv_router/push_router/request_guard.rs
  • lib/llm/src/kv_router/push_router/selection.rs
  • lib/llm/src/session_affinity/coordinator.rs
  • lib/llm/src/session_affinity/mod.rs
  • lib/llm/src/session_affinity/push_router.rs
  • lib/llm/src/session_affinity/tests.rs
  • lib/runtime/Cargo.toml
  • lib/runtime/src/discovery/kube.rs
  • lib/runtime/src/discovery/kv_store.rs
  • lib/runtime/src/discovery/mod.rs
  • lib/runtime/src/pipeline/network/egress/push_router.rs
  • lib/runtime/src/storage/kv.rs
  • lib/runtime/src/storage/kv/file.rs
  • tests/router/common.py
  • tests/router/test_router_e2e_with_mockers.py
💤 Files with no reviewable changes (3)
  • lib/runtime/src/storage/kv.rs
  • lib/runtime/src/pipeline/network/egress/push_router.rs
  • lib/runtime/src/discovery/mod.rs

Comment thread lib/runtime/src/storage/kv/file.rs Outdated
Comment thread lib/llm/src/kv_router/push_router.rs Outdated
Comment thread lib/llm/src/kv_router/push_router.rs Outdated
PeaBrane added 2 commits July 3, 2026 13:59
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
@PeaBrane
PeaBrane enabled auto-merge (squash) July 3, 2026 21:08
@PeaBrane
PeaBrane merged commit 91376af into main Jul 6, 2026
99 checks passed
@PeaBrane
PeaBrane deleted the rupei/revert-distributed-session-affinity branch July 6, 2026 16:36
@PeaBrane PeaBrane changed the title revert(router): keep session affinity local revert(router): keep session affinity local [DYN-3249] Jul 20, 2026
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 revert router Relates to routing, KV-aware routing, etc. size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants