Skip to content

Register the engines with the router from the rollout process instead of ray actors - #1862

Open
fzyzcjy wants to merge 1 commit into
tom/refactor-miles/op3-19from
tom/refactor-miles/op3-18
Open

Register the engines with the router from the rollout process instead of ray actors#1862
fzyzcjy wants to merge 1 commit into
tom/refactor-miles/op3-19from
tom/refactor-miles/op3-18

Conversation

@fzyzcjy

@fzyzcjy fzyzcjy commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Part of #1837

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

@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch from 1ddfff5 to b86f9cd Compare August 3, 2026 23:44
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-3 branch from 28437a3 to 51e1316 Compare August 4, 2026 00:39
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch 2 times, most recently from 32288a2 to 80e518c Compare August 4, 2026 06:57
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-3 branch 2 times, most recently from 5c849ef to cefbc28 Compare August 6, 2026 07:03
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch 2 times, most recently from ad04c0f to 690860c Compare August 10, 2026 06:07
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-3 branch from cefbc28 to 30e603f Compare August 10, 2026 06:07
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch from 690860c to 41fde51 Compare August 10, 2026 16:07
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-3 branch from 282b8b1 to 7db556d Compare August 10, 2026 16:34
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch 2 times, most recently from 2c55eca to 868e4bf Compare August 12, 2026 02:48
@fzyzcjy
fzyzcjy changed the base branch from tom/refactor-miles/op3-3 to tom/refactor-miles/op3-19 August 12, 2026 02:50
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch from 868e4bf to c9c00ee Compare August 12, 2026 14:31
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-19 branch 2 times, most recently from 8b9a268 to aebc0f6 Compare August 12, 2026 23:56
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch from c9c00ee to 1c53279 Compare August 12, 2026 23:56
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-19 branch from aebc0f6 to c20738b Compare August 13, 2026 01:13
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch from 1c53279 to bc4ef91 Compare August 13, 2026 01:13
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-19 branch from c20738b to f40e568 Compare August 13, 2026 03:25
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch 2 times, most recently from aed73d2 to e37f7fd Compare August 13, 2026 06:01
… of ray actors

SGLangEngine registered itself with the router inside init and unregistered
inside shutdown, which is the last thing on it that is not launching a server:
it needed the router address, the legacy-api decision and its own node rank
just to publish a url the rollout process already derives from the allocator.

ServerGroup now publishes its node-0 engines once their servers answer, and
withdraws them in stop_engines before the actor is killed. The engine keeps
nothing router-related, and use_legacy_router_api moves next to the client it
decides for.

Two behaviour changes worth naming: registration now happens after every engine
in a group has initialized rather than per engine inside init, and
simulate_crash no longer withdraws the worker from the router first -- which is
what a real crash does anyway.
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-19 branch from afa0642 to 3b12cee Compare August 14, 2026 04:46
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op3-18 branch from e37f7fd to 0b0f59d Compare August 14, 2026 04:46

@guapisolo guapisolo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving this PR in the context of the #2176 safe-merge checkpoint. Moving router-membership ownership into the rollout process is the right direction, but the original ordering could mark an engine alive before router registration succeeded. #2090 commits serving state only after successful router publication; that fix lands before #2176 and remains present through the checked chain tip.

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.

2 participants