-
Notifications
You must be signed in to change notification settings - Fork 15
Probabilistic priority queueing #322
Probabilistic priority queueing #322
Conversation
…rove Origin/http offer set perf improve
fetch from upstream - eom
…lenhattan86/scheduler into probabilistic_priority_queueing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First quick pass
src/test/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssignerImplTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second pass
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssignerImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssignerImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssignerImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lenhattan86 There are several commits in this PR that are not related, it must be from a stale branch merge. Once those are fixed, PR LGTM.
right, they are merged from master to fix other issues. However, none of them cause conflicts if you look at the list of changed files. https://github.com/aurora-scheduler/scheduler/pull/322/files |
I am not worried about conflicts, but for commit history and number of commits/messages. Once squashed and merged those older commit messages will also be part of single commit. |
sure will clean them up when merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nits: LGTM
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssigner.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssigner.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssigner.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/aurora/scheduler/scheduling/ProbabilisticPriorityAssigner.java
Outdated
Show resolved
Hide resolved
…ticPriorityAssigner.java Co-authored-by: Renán I. Del Valle <github@ridv.xyz>
…ticPriorityAssigner.java Co-authored-by: Renán I. Del Valle <github@ridv.xyz>
…ticPriorityAssigner.java Co-authored-by: Renán I. Del Valle <github@ridv.xyz>
…ticPriorityAssigner.java Co-authored-by: Renán I. Del Valle <github@ridv.xyz>
Description:
The change enables soft priority queueing for aurora-scheduler.
If pending Task A has higher priority than other pending tasks, it has more chance to be scheduled than other pending tasks.
However, it does not mean that low priority tasks have to wait until all higher priority tasks scheduled. So, we do not starve any pending tasks.
The chance of scheduling is calculated using an exponential function p^exp where p is the priority and exp is the exponent.
We control exp using
probabilistic_priority_assigner_exponent
.If exp is 0, there is no priority queueing.
if exp is high, there is less chance that the lower priority tasks can be scheduled.
Testing Done: