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

[Pulsar Scheduler] Don't Schedule Jobs When Executors Are Lagging Behind #2308

Merged
merged 11 commits into from
Apr 16, 2023

Conversation

d80tb7
Copy link
Collaborator

@d80tb7 d80tb7 commented Mar 31, 2023

┆Issue is synchronized with this Jira Task by Unito

@JamesMurkin
Copy link
Contributor

@d80tb7

Is this good to review? I made a minor change to the unit test as it was failing - but looks like it was just a copy paste error

@codecov
Copy link

codecov bot commented Apr 14, 2023

Codecov Report

Patch coverage: 71.42% and project coverage change: +0.01 🎉

Comparison is base (b405422) 58.97% compared to head (ccbeb2c) 58.98%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2308      +/-   ##
==========================================
+ Coverage   58.97%   58.98%   +0.01%     
==========================================
  Files         225      225              
  Lines       28470    28498      +28     
==========================================
+ Hits        16790    16810      +20     
- Misses      10388    10394       +6     
- Partials     1292     1294       +2     
Flag Coverage Δ
armada-server 58.98% <71.42%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/scheduler/scheduling_algo.go 67.22% <71.42%> (+0.43%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@d80tb7
Copy link
Collaborator Author

d80tb7 commented Apr 14, 2023

@JamesMurkin yup I think this one is god to review

@@ -75,6 +75,7 @@ scheduling:
memory: 1.0
cpu: 1.0
maximumJobsToSchedule: 5000
maxUnacknowledgedJobsPerExecutor: 2500
Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly we should re-adjust numbers in config.

We allow scheduling 5000 per round, but only allow 2500 lag - which can easily happen in a single round

Possibly maximumJobsToSchedule should be 1000 instead

@JamesMurkin JamesMurkin merged commit 423273e into master Apr 16, 2023
@JamesMurkin JamesMurkin deleted the f/chrisma/pulsar-scheduler-out-of-sync branch April 16, 2023 13:47
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.

None yet

2 participants