Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing unfeasible scheduling keys bug (#182) #3845

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

MustafaI
Copy link
Contributor

  • Fixing unfeasible scheduling keys bug

  • Renaming test parameter to be more consistent with other parameters

  • Renaming constraints

This is a fix for a bug where unfeasibleSchedulingKeys was being populated incorrectly:

  • This bug occurs when we hit a constraint in gang_scheduler.go, which leads to all jobs of a particular shape being banned for the current scheduling round.
  • We fixed this bug by doing the following:
    • Only add a scheduling key to unfeasibleSchedulingKeys if the job's constraint means it's globally unschedulable (see: internal/scheduler/gang_scheduler.go)
    • In fixing this bug we had to modify some tests in internal/scheduler/queue_scheduler_test.go. Effectively, what the tests used to see as unsuccessful jobs are now seen as not considered. The jobs were in reality never considered and only seen as unsuccessful because of an optimisation done for jobs with scheduling keys seen in unfeasibleSchedulingKeys.
  • We added some tests to internal/scheduler/gang_scheduler_test.go to ensure that unfeasibleSchedulingKeys was being populated for the right reason. Much of the diffs in this file are just reformatting.

mustafai-gr and others added 2 commits July 29, 2024 11:06
* Fixing unfeasible scheduling keys bug

* Renaming test parameter to be more consistent with other parameters

* Renaming constraints
@MustafaI MustafaI merged commit 8a64c0f into master Jul 29, 2024
20 checks passed
@MustafaI MustafaI deleted the sendToGitHub/fix-unfeasibleSchedulingKeys-bug branch July 29, 2024 10:38
This pull request was closed.
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.

3 participants