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

tests: put (back) plugins tests in a single linkFarm #2015

Closed
wants to merge 1 commit into from

Conversation

GaetanLepage
Copy link
Member

No description provided.

@MattSturgeon MattSturgeon marked this pull request as draft August 12, 2024 11:15
@MattSturgeon
Copy link
Member

Thanks for testing this out. I'll try and dig up the command we were using to check memory usage.

I've marked it as a draft because we'll also need to re-work the tests devshell command and probably revert 71126bf too.

@MattSturgeon
Copy link
Member

If we need to, how complex do you think it'll be to split the tests over a handful of link farm derivations? Either based on test-sources/* paths or arbitrarily distributed evenly?

@zowoq
Copy link
Contributor

zowoq commented Aug 13, 2024

With our new hardware a single linkfarm by itself won't OOM but for the couple of minutes it takes to evaluate it won't leave much for everyone else. It is an option but only just, and it can't really afford to get much bigger.

I'll try and dig up the command we were using to check memory usage.

#1878 (comment)

split the tests over a handful of link farm derivations? Either based on test-sources/* paths or arbitrarily distributed evenly?

The command I gave was only using a single worker as sequential eval made it easier to see the problem locally. On the old hardware we were using 16 workers for eval, now we are using 32. Only splitting the linkfarm up a bit (e.g. into 4) likely won't help much as they will still end up being evaluated at the same time but by more than that or by path should work.

I'm hoping we'll have some improvements land in buildbot-nix over the next couple of weeks, my short term solution to speed things up would be to reduce the amount of tests by platform, e.g. full tests on x86_64-linux / aarch64-darwin and only a smoke test on aarch64-linux / x86_64-darwin.

@MattSturgeon
Copy link
Member

With our new hardware a single linkfarm by itself won't OOM but for the couple of minutes it takes to evaluate it won't leave much for everyone else. It is an option but only just, and it can't really afford to get much bigger.

Thanks for clarifying! In that case I don't think we want to go back to a single link farm until buildbot is able to handle it without (potential) issue.

I'll try and dig up the command we were using to check memory usage.

#1878 (comment)

Thanks!

split the tests over a handful of link farm derivations? Either based on test-sources/* paths or arbitrarily distributed evenly?

The command I gave was only using a single worker as sequential eval made it easier to see the problem locally. On the old hardware we were using 16 workers for eval, now we are using 32. Only splitting the linkfarm up a bit (e.g. into 4) likely won't help much as they will still end up being evaluated at the same time but by more than that or by path should work.

Currently we split the tests up one-per file (i.e. test-sources/**), this reduces eval memory a lot, however the quantity of builds it results in leads to a number of other issues:

  • nix flake show takes around 45s and produces a lot of output.
  • the buildbot web UI becomes sluggish and struggles to deal with the quantity of builds it needs to get metadata for
  • buildbot is unable to run all queued builds simultaneously, leading to much slower builds where many builds are sat waiting for an available builder

Only splitting the linkfarm up a bit (e.g. into 4) likely won't help much

Are you saying we'd need in the region of 32 items being built by nix flake check?

test-sources/* won't help much there:

$ ls -ld tests/test-sources/*
drwxr-xr-x 1 matt users  20 Aug 13 13:31 tests/test-sources/example-configurations/
-rw-r--r-- 1 matt users 298 Aug 13 13:31 tests/test-sources/examples.nix
-rw-r--r-- 1 matt users 429 Aug 13 13:32 tests/test-sources/extended-lib.nix
drwxr-xr-x 1 matt users 348 Aug 13 13:32 tests/test-sources/modules/
drwxr-xr-x 1 matt users 258 Aug 13 13:31 tests/test-sources/plugins/

However ls -ld tests/test-sources/*/* prints 32 lines. For contrast ls -ld tests/test-sources/** prints 306 lines!

We also have (a handful of) other checks that aren't under test-sources, so we wouldn't have to hit the number exactly.

I'm hoping we'll have some improvements land in buildbot-nix over the next couple of weeks, my short term solution to speed things up would be to reduce the amount of tests by platform, e.g. full tests on x86_64-linux / aarch64-darwin and only a smoke test on aarch64-linux / x86_64-darwin.

Personally, I'd like a way to indicate to buildbot whether a PR is ready for a "full" check or should do a "light" check (e.g. only on x86_64-linux).

This would fit well with mergify's "two-step CI" strategy, where lightweight checks are run on all PR pushes, but heavy checks are only run on PRs queued for merging.

Queued PRs could be identified with a label, for example.

@zowoq
Copy link
Contributor

zowoq commented Aug 14, 2024

Sorry, just realised that you were asking about this on matrix and I didn't reply. If you don't get a response after a couple of days feel free to ping me.


nix flake show takes around 45s and produces a lot of output.

Yeah, using #checks is not really ideal for a project of the size of nixvim, I'd open an upstream issue about this.

buildbot is unable to run all queued builds simultaneously, leading to much slower builds where many builds are sat waiting for an available builder

Smarter scheduling will help but we only have limited darwin capacity so that will be a problem regardless. Also we still have another two CI systems so other builds could be running even if there isn't a buildbot queue.

Only splitting the linkfarm up a bit (e.g. into 4) likely won't help much

Are you saying we'd need in the region of 32 items being built by nix flake check?

We also have (a handful of) other checks that aren't under test-sources, so we wouldn't have to hit the number exactly.

This PR has 72 items in total but the 4 linkfarms (1 per platform) end up in memory at the same time as the small jobs are processed quicker. 16 linkfarms (4 per platform) probably won't change much, they'll still be the slowest jobs that'll accumulate in memory.

Personally, I'd like a way to indicate to buildbot whether a PR is ready for a "full" check or should do a "light" check (e.g. only on x86_64-linux).

This would fit well with mergify's "two-step CI" strategy, where lightweight checks are run on all PR pushes, but heavy checks are only run on PRs queued for merging.

I'd open an upstream issue about this as well.

@GaetanLepage
Copy link
Member Author

Thank you very much for those insights @zowoq.
One solution could be to fall back to some imperative shell script which would force the CI process to happen somehow sequentially.
This would artificially limit the amount of eval/builds that can happen simultaneously.

Of course, I would rather not have to do that and keep the exhaustive .#checks approach.
This turns out to be a not so easy problem !

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.

3 participants