max-substitution-jobs setting
#8299
Conversation
|
Idea seems good to me! |
urbas
force-pushed
the
max-substitution-jobs
branch
from
b04ed20
to
613bc69
Compare
roberth
added
feature
github-actions
bot
removed
the
store
|
Can see plenty of concurrency with |
| if (goal->jobCategory() == JobCategory::Substitution) { | ||
| assert(nrSubstitutions > 0); | ||
| nrSubstitutions--; | ||
| } else { | ||
| assert(nrLocalBuilds > 0); | ||
| nrLocalBuilds--; | ||
| } |
There was a problem hiding this comment.
Now that we have the enum, it is better to do a switch here. Note because of some -Werror=... you might need a default: assert(false);
There was a problem hiding this comment.
Even better would be something like
nrActiveGoals[goal->jobCategory()]--;
There was a problem hiding this comment.
I feel like that bakes in an assumption that resources are independent. Relatively speaking; it's just code, and I think it's all fine.
| bool isSubstitutionGoal = goal->jobCategory() == JobCategory::Substitution; | ||
| if ((!isSubstitutionGoal && getNrLocalBuilds() < settings.maxBuildJobs) || | ||
| (isSubstitutionGoal && getNrSubstitutions() < settings.maxSubstitutionJobs)) |
There was a problem hiding this comment.
Also good to use switch and handle each case explicitly.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/content-addressed-nix-call-for-testers/12881/214 |
Motivation
Addresses #3379.
A good example of why
max-substitution-jobsis needed is the case where we setmax-jobsto 0 to force derivations to be built on remote stores. Unfortunately, this limits the number of concurrent substitutions to 1 (not 0 because there's an extra provision to allow at least 1 substitution goal in the case whenmax-jobsis set to 0). This slows down substitutions.Another motivating example is the fact that
max-jobsdefault is set to1, which means that substitution parallelism will be1by default. Again, this unnecessarily slows down substitutions.Context
Currently the maximum number of concurrent substitutions is determined by the
max-jobssetting. This PR is in agreement with issue #3379 and proposed a new setting named--max-substitution-jobs.Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.shsrc/*/teststests/nixos/*Priorities
Add 馃憤 to pull requests you find important.
cc: @domenkozar @Mic92