Debate what the default number of VCPUs should be

[grahamwhaley]

Currently we set the default number of VCPUs to be 1:
https://github.com/containers/virtcontainers/blob/master/hypervisor.go#L36

This is one obvious and safe default, but from some testing (under https://github.com/clearcontainers/runtime), it seems that running up the VM (qemu/KVM) with only one VCPU causes some performance issues (startup takes twice as long for instance). (side note - this happened as the Clear Containers cc-runtime failed to find a valid config file upon startup during some debug, so took the default value for number of VCPUs from virtcontainers....).
Thus, we should debate if this needs changing or not. Obvious options are:

• leave it as is - it is a known limitation, and runtimes should be choosing their own appropriate defaults
• set it to '2', as the next best 'safe' choice
• have it dynamically set to the number of physical cpu/cores in the host system. For very large systems though this may not be a great default choice due to the cost of creating and running all the VCPUs in the VM.

[dvoytik]

IMHO runtime should define vCPUs default number if we consider virtcontainers as a library.

[grahamwhaley]

@dvoytik I agree, I think runtimes should have their own idea of a default vCPUs if they have a 'good notion' of what that should be. We should probably treat the defaultVCPUs in virtcontainers as a 'safe fallback recommendation' - or have no defaults in virtcontainers at all, and hoist the responsibility to do the right thing out to the runtimes.
As I'm pretty sure you are aware, it then comes down to runtime code such as this:
https://github.com/clearcontainers/runtime/blob/master/config.go#L154
to make a choice, or fault if it cannot work out what the user has requested or find a configured default.
It would seem a little remiss for us to have latent knowledge (like VCPUs==1 is not a great idea), and not code that into the library though.
/cc @sboeuf

[sboeuf]

I agree with both of you. The runtime has to be able to decide what he wants, and this is already an available configuration option, thus nothing to do for that part.
About virtcontainers, I think that we should go with kind of an hybrid solution:

const (
        minVCPUs = 2
        maxVCPUs = 8
)
defaultVCPUs := systemNumProc / 2

if defaultVCPUs < minVCPUs {
        defaultVCPUs = minVCPUs
} else if defaultVCPUs > maxVCPUs {
        defaultVCPUs = maxVCPUs
}

[grahamwhaley]

Given we have clearcontainers/runtime#297 (thanks @dvoytik ;-), so I believe we have the facility to over-ride the defaults... I like the above proposed code snippet.
Two things occur to me:

1. Hmm, this will (potentially) give different default behavior on different machines. I'm not sure I like that... @mcastelino any thoughts there?
2. If we do (even if we do not) implement this - is there somewhere in virtcontainers we can document it? I don't see anywhere leaping out at me. I expect we should have godocs, and a nice link off the github pages. @sboeuf let me know if there are plans around that area already. /cc @jodh-intel and @egernst for their input here - let me know, and I'll open some Issues if we need.

[sboeuf]

@grahamwhaley godoc sounds good to me. There is no maintenance, everything is generated automatically here https://godoc.org/github.com/containers/virtcontainers.

[mcastelino]

@sboeuf @grahamwhaley we can choose a sane number as the default. This can be 2. However the runtime should set the number of vCPUs == number of system physical CPUs to approximate runc behavior. If the user wants to constrain the same, we should honor it once we support the cpu and cpu set options.

[sboeuf]

@mcastelino just to make sure I understand properly, you're suggesting only a minimal value, but no maximal value, right ?

[mcastelino]

@sboeuf yes. The library should not bound the maximal value. The runtime should be able to choose any number. Also is there a reason to bound the minimal value? Yes to boot up will be slow if the value is set to 1. But but setting it to 2, when the value requested is 1, also does not quite make sense. Specially if the user is more concerned about constraining the program.

One of the advantages clear containers has over a namespaced container is the ability to actually control the visibility of the CPUs (and for that matter memory)

For example in the case of runc

docker run --cpus 2 -it debian sh -c "grep -c ^processor /proc/cpuinfo"

Will yield more than 2 cpu's.

With clear containers we have the ability to limit to explicitly the number of cpus specified in --cpu.

This allows a certain class of applications that look at such information to decide the number of threads to spawn and other characteristics to run in clear container, to run unmodified from their original design from a VM world.

[sboeuf]

@mcastelino well the thing is that as long as we get this value (number of CPUs) from the runtime, we will use it.
But the discussion is more about what could be the default values if the runtime does not set anything specific. In that case, we want virtcontainers to set something.

[mcastelino]

@sboeuf that helps. So what does the runtime send to the library today when docker/kubernetes leave the --cpu unspecified. If they do not send anything, the right behavior of virtcontainer should be IMO

defaultVCPUs := systemNumProc

or

defaultVCPUs := systemNumProc / numNumaNodes

or

defaultVCPUs := systemNumProc / numSockets

defaultVCPUs := systemNumProc / 2

seems arbitrary.