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

fix: the scheduler is now fair #2158

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Conversation

andaaron
Copy link
Contributor

@andaaron andaaron commented Jan 8, 2024

Generators are now ordered by rank in the priority queue.

The rank computation formula is:
- 100/(1+generated_task_count) for high priority tasks
- 10/(1+generated_task_count) for medium priority tasks
- 1/(1+generated_task_count) for low priority tasks

Note the ranks are used when comparing generators both with the same priority and with different priority.
So now we are:
- giving an opportunity to all generators with the same priority to take turns generating tasks
- giving roughly 1 low priority and 10 medium priority tasks the opportunity to run for every 100 high priority tasks running.

After a generator generates a task, the generators are reordered in the priority queue based on rank.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

codecov bot commented Jan 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (355b1ee) 92.15% compared to head (aa21bd8) 92.20%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2158      +/-   ##
==========================================
+ Coverage   92.15%   92.20%   +0.05%     
==========================================
  Files         166      166              
  Lines       28770    28792      +22     
==========================================
+ Hits        26513    26549      +36     
+ Misses       1662     1652      -10     
+ Partials      595      591       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andaaron andaaron force-pushed the scheduler-fix branch 2 times, most recently from 3ea716b to 2c707f4 Compare January 11, 2024 16:20
@andaaron andaaron changed the title Scheduler updates fix: the scheduler is now fair Jan 11, 2024
@andaaron andaaron marked this pull request as ready for review January 11, 2024 16:25
@andaaron andaaron force-pushed the scheduler-fix branch 6 times, most recently from 1f3683a to 1e61713 Compare January 17, 2024 15:30
Generators are now ordered by rank in the priority queue.

The rank computation formula is:
- 100/(1+generated_task_count) for high priority tasks
- 10/(1+generated_task_count) for medium priority tasks
- 1/(1+generated_task_count) for low priority tasks

Note the ranks are used when comparing generators both with the same priority and with different priority.
So now we are:
- giving an opportunity to all generators with the same priority to take turns generating tasks
- giving roughly 1 low priority and 10 medium priority tasks the opportunity to run for every 100 high priority tasks running.

After a generator generates a task, the generators are reordered in the priority queue based on rank.

Signed-off-by: Andrei Aaron <[email protected]>
@rchincha
Copy link
Contributor

So this is a dynamic heuristic -
Between generators of constant output rate, numerator determines priority and when one of them has a higher rate, priority decreases.

Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Tested in production, memory/cpu usage looking much better.

@rchincha rchincha merged commit 8215766 into project-zot:main Jan 25, 2024
33 checks passed
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