Repository navigation
Conversation
k82cn
force-pushed
the
feat/independent-warmup
branch
from
October 27, 2026 06:24
d7844b0 to
2ad855f
Compare
k82cn
force-pushed
the
feat/independent-warmup
branch
from
October 27, 2026 07:00
a29ca43 to
e3b5b75
Compare
This was referenced Oct 27, 2026
Contributor
Author
|
Superseded by #636: the independent warm-up design was dropped in favor of tying a gang's min_instances and max_instances to its batch_size. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
min_instancesis a warm-up andbatch_sizeis a gang, and the design (RFE591, 4.5 and 4.6) says they are met independently. Dispatch did not do that: a session with pending tasks bound everything through oneStatement, and when the gang could not be formed it discarded the whole statement, warm-up executors included. Whether the warm-up held its executors then depended on whether a task happened to be pending when the scheduler ran. That is what madeTestGangScheduling::test_single_instance_sessions_are_not_held_backfail intermittently (the gang session was warmed up one executor at a time in the gap betweencreate_sessionandcreate_task, then could not get a seventh; see #636 for the test side).Change
MinMax::is_underusedno longer counts the gang size: it compares the executors a session holds withtask_demand,min_demandandmax_demand(new, with the same bounds as before).compute_demandis renamedsession_demand; it adds the gang size while a task is pending and is unchanged foris_ready,is_overusedand DRF. A gang is formed all at once byis_satisfied, not one executor at a time.DispatchAction(scheduler/actions/dispatch.rs) first dispatches the warm-up: for a session withmin_instancesabove 0, whileis_underusedholds (the executors it has are below what its tasks andmin_instancescall for, within the instance limits), it binds eligible Idle executors, in aStatementof its own that is always committed. It stops at the task demand and not atmin_instances, so withmin_instancesabove 0 a gang whose tasks exceed the cluster can hold a partial set; whether the cluster has the resources is up to the user. Withmin_instances0 the gang stays all-or-nothing. Then the existing gangStatementruns; the executors just bound count toward the gang, and when the gang cannot be formed only its own binds are discarded. No new plugin hook and no executor counting in Dispatch.Behavior change
Executors are created (Allocate) for the session's tasks and
min_instancesonly, so a gang needs one pending task per member: a gang of 4 with one pending task is no longer given four executors. The examples and the e2e tests submitbatch_sizetasks; RFE591 4.2 anddocs/api/types.mdsay so.A session that sets
min_instancesandbatch_sizeto the same number no longer gets an all-or-nothing group: the warm-up holds what it can, and a bound executor takes a pending task whether or not the gang is complete. The scheduler does not hold a gang's tasks back for it; the service has to make sure of that. For an all-or-nothing group, setbatch_sizealone. In the repository,examples/rl/search-r1already does this (trainer.pyusesbatch_size,rollout.pyuseswarmup); the RFE584 text still said the trainer usedwarmup=N, so it is corrected.docs/api/types.md,sdk/python/docs/API.mdand RFE591 (new 4.3.1, the Dispatch steps, the edge cases and the test plan) describe the behavior.Tests
plugins/minmax.rs:is_underusedcounts the tasks and the warm-up but not the gang.scheduler/mod.rs, with real controller storage and a cycle loop, bothpriority+dasanddrf+das: a gang of 7 withmin_instances7 on 6 slots holds 6, and withmin_instances0 holds none; a gang of 4 withmin_instances2 and one task holds 2 on 3 slots, and with four tasks 4 on 4;min_instances3 without tasks binds exactly 3. The first two fail onmain'sdispatch.rs(they get 0 executors).test_competing_gangs_never_hold_a_partial_setnow creates its gangs withoutmin_instances: withmin_instancesequal to the gang size, one gang holds 4 and the other 2 by design.cargo test -p flame-session-manager(300 passed with--test-threads=6; at the default parallelism some tests hit the per-process file limit on my machine), clippy-D warnings, rustfmt.testing/e2e/tests/test_session.pyhas the same change as fix: a gang's min_instances is 0 or batch_size and a kept-warm gang binds together #636 (gangs created withbatch_sizeonly and one long task per executor, becausemin_instancesequal to the gang size would now hold executors by design). fix: a gang's min_instances is 0 or batch_size and a kept-warm gang binds together #636's CI showed that change needs another fix (a session with running tasks cannot be closed), which this pull request does not have yet.