Skip to content

Refactor to remove allocate_rollout_engine_addr_and_ports_normal - #1875

Open
fzyzcjy wants to merge 1 commit into
tom/refactor-miles/op5-11from
tom/refactor-miles/op5-10
Open

Refactor to remove allocate_rollout_engine_addr_and_ports_normal#1875
fzyzcjy wants to merge 1 commit into
tom/refactor-miles/op5-11from
tom/refactor-miles/op5-10

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/op5-10 branch 2 times, most recently from 8049dac to dc38c1c Compare August 3, 2026 23:44
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from 00d7f71 to c670bc3 Compare August 3, 2026 23:44
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from dc38c1c to cdc8c1f Compare August 4, 2026 00:39
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from c670bc3 to e5d3ce9 Compare August 4, 2026 00:39
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from cdc8c1f to 23c20fa Compare August 4, 2026 06:57
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from e5d3ce9 to f9e2f2c Compare August 4, 2026 06:57
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch 2 times, most recently from f465bcd to e34f5f9 Compare August 10, 2026 06:07
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch 2 times, most recently from 5a8aa57 to ae9ef9b Compare August 10, 2026 16:07
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from e34f5f9 to 27f7dce Compare August 10, 2026 16:07
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from ae9ef9b to 8c1cc7a Compare August 10, 2026 16:34
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch 2 times, most recently from 1fb4d92 to d31c66c Compare August 12, 2026 02:48
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from 74bb84d to 312d013 Compare August 12, 2026 14:31
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch 2 times, most recently from 310dda2 to 979f051 Compare August 12, 2026 23:56
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from 7c11f3d to 5249f62 Compare August 13, 2026 01:13
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from 979f051 to 12bc976 Compare August 13, 2026 01:13
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from 12bc976 to 80f1dd4 Compare August 13, 2026 03:25
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-11 branch from 31561c5 to f8131a2 Compare August 13, 2026 06:01
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from 80f1dd4 to fd62164 Compare August 13, 2026 06:01
allocate_rollout_engine_addr_and_ports_normal is inlined into
ServerGroup.start_engines: the cell list is derived from new_engine_indices,
ports are probed via the underlying actor handle, and the padding that filled
addressing entries up to the end of each node is dropped, since only the
newly launched engines are ever initialized from it. addr_and_ports stays
keyed by global rank, as before. The addressing tests drive
ServerGroup.start_engines now that the allocation function they called is
gone.

disaggregation_bootstrap_port stays prefill-only, so a regular or decode
engine neither reserves a port for it nor reports a bootstrap_port in its
AddrInfo.
@fzyzcjy
fzyzcjy force-pushed the tom/refactor-miles/op5-10 branch from fd62164 to f666bf2 Compare August 14, 2026 04:45

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

Reviewed in the full #1837 stack context. The source-time multi-node rendezvous risk is addressed by #1877’s whole-cell invariant and #1886’s cell-owned state model, both before safe merge checkpoint #2176. No active findings remain for this PR.

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