apptainer / singularity

Singularity has been renamed to Apptainer as part of us moving the project to the Linux Foundation. This repo has been persisted as a snapshot right before the changes.
https://github.com/apptainer/apptainer
Other
2.53k stars 424 forks source link

mksquashfs takes up all cores on build system #1228

Closed gnguy closed 4 years ago

gnguy commented 6 years ago

Version of Singularity:

2.4.2

Expected behavior

singularity build utilizes one core when calling mksquashfs, unless otherwise specified.

Actual behavior

singularity build utilizes all cores available on machine

Steps to reproduce behavior

singularity build with a large (>10 gb gb) image. On a 40+ core machine, this operation takes up all cores available on the machine. I believe this is due to the default settings for the processors option in mksquashfs:

-processors NUMBER Use NUMBER processors. By default will use number of processors available.

Would it be possible to add an option to Singularity build to specify the number of processors used, to pass to mksquashfs when it is called? Otherwise, I'm not sure about straightforward ways to control processor usage -- I'm looking into using numactl as a workaround, but it would be nice to be able to control the cpu usage either via an environmental variable (similar to the SINGULARITY_PYTHREADS environmental variable) or as an explicit option to singularity build.

standage commented 4 years ago

2+ years later, this is still an issue with Singularity 3.4.1. Being unable to configure the number of processors used for singularity build is problematic in a lot of contexts.

standage commented 4 years ago

Would it be as straightforward as updating this code block and appending -processors 1 (or an alternative user-supplied value)?

https://github.com/sylabs/singularity/blob/d642f2bd3e96d77d0fa18611059407fb4f697aa6/pkg/image/packer/squashfs.go#L38-L41

UPDATE: as subsequent discussion has shown, no it is NOT that simple! 🤔

jmstover commented 4 years ago

About ... would probably want to instead do it in internal/pkg/build/assemblers/sif.go in the Assemble function. See where it's doing things like:

flags = append(flags, "-comp", "gzip")

So, we'd probably want to add in a --cpu or --processors build flag to set the -processors option if passed, or just used the default (which is all CPUs).

@cclerget @dctrud Thoughts on this approach?

dtrudg commented 4 years ago

It would be nice to have this in the next release, agreed.

I believe that in order to satisfy the needs which have been expressed here, and in other fora, we would want to:

Although some in the past have asked for a default of 1 thread... this is a poor experience for people using a personal laptop or desktop system - where many container builds happen.

jmstover commented 4 years ago

Add an environment variable (and CLI flag?) to allow the user to set

If we do both a singularity.conf value, and CLI option, then I would want the singularity.conf option to have precedence. Meaning, you couldn't set the CLI option above what is set in the singularity.conf ... So if 6 is set in singularity.conf on the CLI you could set a value from 1-6, but not go above that.

standage commented 4 years ago

I believe that in order to satisfy the needs which have been expressed here, and in other fora, we would want to:

  • Default to the current situation, where we do not restrict the number of cores
  • Expose a configuration value in singularity.conf allowing admins to set a different default for a nice default on e.g. HPC login nodes where all cores shouldn't be use by default.
  • Add an environment variable (and CLI flag?) to allow the user to set

That is all reasonable. I wish I could offer more help, but I don't do much in Go. I'd be happy to do some small, well-defined tasks, but then again I might just get in the way. :-)

standage commented 4 years ago

If we do both a singularity.conf value, and CLI option, then I would want the singularity.conf option to have precedence. Meaning, you couldn't set the CLI option above what is set in the singularity.conf ... So if 6 is set in singularity.conf on the CLI you could set a value from 1-6, but not go above that.

Sounds like there are potentially two values: a "max processors" value and a "default processors" value. The first probably only makes sense as a singularity.conf setting, but the second could be defined in singularity.conf and overridden with the CLI.

If the "default processors" value exceeds the "max processors" value in the singularity.conf file, singularity should throw an error.

If a user uses the CLI to request more processors than the "max processors" value in singularity.conf, this could be handled with a warning message to stderr, something like

WARNING: 20 processors requested, but "max_build_processors" set to 16 in /path/to/etc/singularity/singularity.conf;
    scaling down to 16 processors
dtrudg commented 4 years ago

I'm generally of the opinion that maximum CPU limits to be enforced on users like this should be set using system tools such as cgroups. Within the build process of a container, or when running a container a user could easily consume >N cpus.

cclerget commented 4 years ago

Adding mksquashfs max cpus = X directive to singularity.conf (0 would be the default value and would mean all cpus available) and in parallel adding an environment variable SINGULARITY_MKSQUASHFS_MAX_CPUS which would be honored if mksquashfs max cpus = 0 or if it's > 0 and < to mksquashfs max cpus = X (where X != 0). We could also add a note about that this value may be overriden by the number of available cpus.

Does that sounds reasonable ?

standage commented 4 years ago

That's a lot to unpack, let me make sure I understand.

  1. Add new mksquashfs max cpus = X directive to singularity.conf; by default X=0 meaning use all CPUs
  2. Add new environmental variable SINGULARITY_MKSQUASHFS_MAX_CPUS
    1. If unset, default to singularity.conf setting
    2. If set to some integer value Y
      1. Y == 0, default to X (singularity.conf setting)
      2. 0 < Y ≤ X, honor environmental variable and use Y CPUs
      3. Y > X, default to X, possibly with warning message to STDERR

If I've captured that correctly, then yes, I think it's reasonable. Will probably need some error handling for stupid inputs (sysadmin sets max CPU limit to 3.14159, user sets environmental variable to -1, etc).

cclerget commented 4 years ago

@standage Yes and no worries regarding value parsing because it's already handled

dtrudg commented 4 years ago

Closed via #5003

standage commented 4 years ago

Awesome. Thanks for your response on this!

dtrudg commented 4 years ago

Awesome. Thanks for your response on this!

Thanks go out to .@dmbeeson who contributed the PR :-)