Properly conclude scheduling if there are no jobs #6826
Conversation
| "Scheduling: Free workers: $free_worker_count/$worker_count; Scheduled jobs: " . scalar(keys %$scheduled_jobs)); | ||
|
|
||
| my ($allocated_workers, $allocated_jobs) = $self->_allocate_jobs($free_workers); | ||
| return [] unless keys %$allocated_workers; |
There was a problem hiding this comment.
OTOH, might also be possible to just delete this line altogether? That turns the loop below into a noop
There was a problem hiding this comment.
Yes, you can also just remove this line entirely (and also not add an additional emit('conclude') relying on the one at the end of the function). That would probably be the best solution.
There was a problem hiding this comment.
I just removed all early exits.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6826 +/- ##
==========================================
- Coverage 99.26% 99.26% -0.01%
==========================================
Files 402 402
Lines 41400 41395 -5
==========================================
- Hits 41094 41089 -5
Misses 306 306 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| my $limit = OpenQA::App->singleton->config->{scheduler}->{max_running_jobs}; | ||
| if ($limit >= 0 && $running >= $limit) { | ||
| log_debug("max_running_jobs ($limit) exceeded, scheduling no additional jobs"); | ||
| $self->emit('conclude'); |
There was a problem hiding this comment.
Don't we still need it here? This condition has an early return and I suppose the same counts here as for other early returns.
There was a problem hiding this comment.
No, the call at the end of schedule is reached in that case.
There was a problem hiding this comment.
Oh, right. This is a whole different function.
| "Scheduling: Free workers: $free_worker_count/$worker_count; Scheduled jobs: " . scalar(keys %$scheduled_jobs)); | ||
|
|
||
| my ($allocated_workers, $allocated_jobs) = $self->_allocate_jobs($free_workers); | ||
| return [] unless keys %$allocated_workers; |
There was a problem hiding this comment.
Yes, you can also just remove this line entirely (and also not add an additional emit('conclude') relying on the one at the end of the function). That would probably be the best solution.
If a wakeup happens while there are no jobs to be scheduled, it did not emit concluded and the timer for scheduling is left at 0ms, which causes high CPU and DB use by the scheduler. Just remove all early exits, which just increased the complexity of the method without much benefit.
Vogtinator
force-pushed
the
schedulerfix
branch
from
852552d to
bef46f5
Compare
| my $limit = OpenQA::App->singleton->config->{scheduler}->{max_running_jobs}; | ||
| if ($limit >= 0 && $running >= $limit) { | ||
| log_debug("max_running_jobs ($limit) exceeded, scheduling no additional jobs"); | ||
| $self->emit('conclude'); |
There was a problem hiding this comment.
Oh, right. This is a whole different function.
2 participants
I found that openqa-schedule was pretty much always at 100% CPU use on my workstation, which could only be worked around by a restart of the scheduler. I finally had a look and this fixes it.
If a wakeup happens while there are no jobs to be scheduled, it did not emit concluded and the timer for scheduling is left at 0ms, which causes high CPU and DB use by the scheduler. Move the call to emit concluded to the right place to ensure it's called in that case as well.