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
Consumable system features #588
base: master
Are you sure you want to change the base?
Conversation
Adds support for consumable system features. Remote build machines can specify a supported or mandatory feature as consumable with a "feature:n" notation, where n is the quantity of the feature that the machine provides. "feature:0" and "feature" are both interpreted as a non-consumable (infinite) feature. Derivations can specify how much of a feature they consume with a similar "feature:n" notation, where n is the quantity of the feature that the derivation consumes. "feature" is interpreted as "feature:1" (i.e., consumes one of the feature), and "feature:0" is interpreted as still requiring the feature but not consuming any of it. Closes NixOS#585 Signed-off-by: Lisanna Dettwyler <lisanna.dettwyler@gmail.com>
don't know how to add people for reviewing... so just tagging @edolstra @shlevy @rbvermaa Intent is that this would also make its way into Nix eventually... either when I have time or probably @edolstra has time. We should make sure that the notation especially wouldn't break anything in Nix before the hydra part gets merged (assuming people actually think this is useful enough to merge :^)) Any objections to the hydra part getting merged before the corresponding Nix patch is ready? This shouldn't break anything in terms of backwards compatibility... Nix already doesn't allow ":" characters in system feature names, so AFAIK no one is using it already incidentally for some other meaning. |
I'd be very happy if this could get reviewed, I've been looking for pretty much exactly like this change for work. |
@ledettwy Thanks a lot for this! I've rebased this PR onto master and also added more status reporting in this branch: Thanks to consumable features, my NixOS tests can specify how many CPUs they use. Hydra will schedule them on build slaves. Without this feature, these tests often "timeouts" because there is not enough CPUs to run them. There is still an issue on the Nix side since the Nix configuration must contain all feature amounts:/ For example, in a NixOS test deviation, I have:
The
And the
With this configuration, Hydra never schedule more than 4 NixOS tests in parallel, even if the max jobs number is 8. |
Derivation can specify required features. Hydra supports consumable features [1] by specifying an amount as suffix feature. Basically, a derivation can specify a `requiredSystemFeature` `feature:N` meaning this derivation requires `N` units of the feature `feature`. The Hydra scheduler schedules jobs on Nix slaves by respecting these constraints. The available amounts of features on slaves are specified in the `machines` file. Currently, before building a derivation, Nix checks that all required features of this derivation are provided (they are specified via the `system-features` option). Since the derivation needs `feature:N`, `feature:N` must be specified in the `system-features` option list. This is not convenient since the value of `N` can vary! This patch proposes to ignore the suffix `:N` in the `requiredSystemFeature` attribute of deviations. This allows Nix to build derivations specifying consumable features by only specifying the `feature` (and not `feature:N`) in the `system-features` list. A warning is printed (`Chatty` mode) to inform the user consumable feature amount is ignored. "warning: the amount of the consumable feature 'cpu:4' is ignored in derivation '/nix/store/ndf6b9r8wb6ljci3k1w3jddrsrscgm7d-vm-test-run-tcp-flows.drv' Note this could break backward compatibility for users that are using a `requiredFeatures` containing a `:`. [1] NixOS/hydra#588
The `kvm` feature keyword is repeated by the number of core of each tests. Hydra then ensures it has enough cores at scheduling time. See NixOS/hydra#588 for details.
The `kvm` feature keyword is repeated by the number of core of each tests. Hydra then ensures it has enough cores at scheduling time. See NixOS/hydra#588 for details.
The `kvm` feature keyword is repeated by the number of core of each tests. Hydra then ensures it has enough cores at scheduling time. See NixOS/hydra#588 for details.
FYI, I finally added the support to specify consumables feature by repeating the feature keyword in d6d31ac. For instance To have a correct support of consumable features in Hydra, I think we have to patch Nix to either support consumable features, or ignore them... because right now, it's a bit hacky! |
The `kvm` feature keyword is repeated by the number of core of each tests. Hydra then ensures it has enough cores at scheduling time. See NixOS/hydra#588 for details.
+1 for this feature. We also need this for controlling scheduling tests onto physical hardware that only allow one job at a time. |
Adds support for consumable system features.
Remote build machines can specify a supported or mandatory feature as
consumable with a "feature:n" notation, where n is the quantity of the
feature that the machine provides. "feature:0" and "feature" are both
interpreted as a non-consumable (infinite) feature.
Derivations can specify how much of a feature they consume with a
similar "feature:n" notation, where n is the quantity of the feature
that the derivation consumes. "feature" is interpreted as "feature:1"
(i.e., consumes one of the feature), and "feature:0" is interpreted as
still requiring the feature but not consuming any of it.
Closes #585
I tested this locally with a variety of different feature combinations and it behaves correctly.
Also, I am not a C++ programmer. A lot of this may very well be written in a non-optimal or non-idiomatic way. If there's a better way to write any of the code I've got here, please let me know.
Design notes are at NixOS/nix#2307, feedback / discussion on the design should happen there.
There's a corresponding implementation that should go into NixOS/nix, but that will probably have to be authored by someone who's more familiar with the codebase than I am.