Skip to content

[2/5] fix(sglang): confirm router worker activation - #1804

Open
ashtonchew wants to merge 2 commits into
radixark:mainfrom
GymPod:ashtoncw/sglang-router-lifecycle
Open

[2/5] fix(sglang): confirm router worker activation#1804
ashtonchew wants to merge 2 commits into
radixark:mainfrom
GymPod:ashtoncw/sglang-router-lifecycle

Conversation

@ashtonchew

@ashtonchew ashtonchew commented Jul 26, 2026

Copy link
Copy Markdown

Summary

  • Makes SGLang engine registration and shutdown wait for confirmed router activation and removal instead of treating request acceptance as completion.
  • Separates engine initialization from router publication and retains the accepted worker identity needed to resume polling or clean up an ambiguous registration.
  • Preserves synchronous Router 0.2.1 and MilesRouter behavior, and still terminates the engine process when router cleanup fails.

Context

Router 0.2.2 and later use the asynchronous worker lifecycle. The existing SGLangEngine path treated a successful registration or removal request as the completed state. That can expose an engine before activation is confirmed, lose the identity required for cleanup, or submit the same operation again after an ambiguous response.

This second layer integrates the typed lifecycle from PR #1801 with engine startup and shutdown. It establishes a callable publication boundary while leaving the Ray recovery ordering to PR #1805.

Stack goal: This five-PR sequence addresses issue #1724: recovered updatable engines remain unroutable until actor-weight synchronization succeeds, and both rollout ingestion paths reject evidence that this invariant was violated.

Layer Upstream PR Status Contract
1/5 #1801 Ready for review Typed asynchronous Router lifecycle
2/5 #1804 Ready for review Confirmed SGLang worker activation and removal
3/5 #1805 Ready for review Recovered-engine publication after weight synchronization
4/5 #1806 Ready for review Direct rollout published-weight guard
5/5 #1807 Ready for review Session-ingress published-weight guard

All public drafts target main because external-fork branches cannot be selected as base refs in this repository. This matches the existing Miles external-stack convention: descendants appear cumulative until their parents merge, then GitHub removes shared commits from later diffs. Merge in order. If a parent is squash- or rebase-merged and commit identities change, rebase the descendants onto the updated main before continuing.

Description

  • Uses the typed lifecycle client for Router 0.2.2 and later while retaining the synchronous Router 0.2.1 and MilesRouter compatibility path.
  • Adds an explicit register_with_router initialization decision so a caller can start an engine without making it routable.
  • Stores accepted worker identity for resumed polling and treats an unknown submission as a generation-level quarantine boundary, preventing a duplicate registration request.
  • Reconciles a Router 0.3.x worker URL to its router-assigned UUID when shutdown must clean up a registration whose response was lost.
  • Waits for confirmed worker absence during shutdown and terminates the engine process in a finally path even when router cleanup raises.

Validation

Layer checks

  • The combined router client and SGLang engine lifecycle suites passed 43 tests.
  • Exact branch-head CPU CI passed all four Stage A shards and Stage B on 4f4056c872.
  • The complete five-layer regression selection passed 112 focused tests.

Stack check

At fixed stack head 5fdef0252e, the recovered engine stayed out of router membership while weight transfer was held. It appeared only after transfer completed and activation was confirmed. The paired baseline and fixed method, results, and proof boundary are documented in PR #1805.

This layer establishes the boundary used by that fix: Miles can start a recovered engine privately and wait until the router confirms activation.

Risks and Follow-ups

Router 0.3.x URL-to-UUID reconciliation is bounded. If an accepted registration never becomes observable before the cleanup deadline, recovery must replace that engine generation with a fresh worker URL rather than retrying an operation with an unknown outcome.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

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.

1 participant