Repository navigation
refactor(scheduler): share the binding driver between bind_task_bias and bind_task_round_robin - #2617
Open
cestercian wants to merge 1 commit into
Open
cestercian wants to merge 1 commit into
cestercian wants to merge 1 commit into
Conversation
…and bind_task_round_robin
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.
Which issue does this PR close?
Rationale for this change
bind_task_biasandbind_task_round_robincopied the same scaffolding (all-zero early return, vcores sort, running job loop, graph lock, skip/blacklist handling,bind_onecall). Only the executor pick was different, and the round robin version had a hand-rolled circular scan with ascannedcounter nested pretty deep.What changes are included in this PR?
bind_tasks_with(budgets, running_jobs, if_skip, strategy)that owns all the shared parts.BindStrategyenum (Bias,RoundRobin) withpick(which executor gets the next task) andadvance(where the next search starts). Bias keeps packing the same executor and only moves forward, round robin moves to the next one and wraps.next_with_capacity(budgets, start)for the circular scan. It returnsNonefor an empty list or when everything is drained, so it no longer relies on the earlier all-zero check to avoid indexing an empty vec.bind_task_biasandbind_task_round_robinkeep their signatures and are now thin wrappers, so callers inmemory.rsandgrpc.rsare untouched. The skip log message keeps the same wording per strategy.Are these changes tested?
Yes. The existing
test_bind_task_bias/test_bind_task_round_robintests still pass unchanged, and I added unit tests fornext_with_capacity(empty, all zero, wraparound, single executor) and for the bias vs round robinpick/advancebehaviour.Are there any user-facing changes?
No, scheduling behaviour is the same.