Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

error in vCPU calculation documentation #489

Closed
egernst opened this issue Jun 8, 2019 · 4 comments · Fixed by #495
Closed

error in vCPU calculation documentation #489

egernst opened this issue Jun 8, 2019 · 4 comments · Fixed by #495
Assignees
Labels
bug Incorrect behaviour needs-review Needs to be assessed by the team.

Comments

@egernst
Copy link
Member

egernst commented Jun 8, 2019

See https://github.com/kata-containers/documentation/blob/master/design/cpu-constraints.md

The formula is listed as:

(quota + (period -1)) / period

This doesn't match the code, and it is not correct. vCPU = quota / period

This is related to kata-containers/runtime#1767

@egernst egernst added bug Incorrect behaviour needs-review Needs to be assessed by the team. labels Jun 8, 2019
@egernst
Copy link
Member Author

egernst commented Jun 8, 2019

/cc @devimc @jcvenegas

@shsha-msft
Copy link

Just to clarify. Is the following understanding correct?

For containers in a POD the formula is vCPU = quota / period
For the POD itself the formula is : (quota + (period -1)) / period

/cc @gkhanna79

@jcvenegas
Copy link
Member

jcvenegas commented Jun 11, 2019

The calculation is as follow:

The containers in a pod are converted to milli cpus

func CalculateMilliCPUs(quota int64, period uint64) uint32 {

        // If quota is -1, it means the CPU resource request is
        // unconstrained.  In that case, we don't currently assign
        // additional CPUs.
        if quota >= 0 && period != 0 {
                return uint32((uint64(quota) * 1000) / period)
        }

        return 0
}

Example:

//  This is 1500
milliCPUs:= CalculateMilliCPUs(150000, 100000)

When a pod has mutiples containers the milliCPUs are sum:

millCPUs := 0
milliCPUs += CalculateMilliCPUs(150000, 100000)
milliCPUs += CalculateMilliCPUs(150000, 100000)
// This result in 3000 millcpus

After the milliCPUs for a POD are calculated this is converted to CPUs with:

//CalculateVCpusFromMilliCpus converts from mCPU to CPU, taking the ceiling
// value when necessary
func CalculateVCpusFromMilliCpus(mCPU uint32) uint32 {
       return (mCPU + 999) / 1000
}

VCPUS:
(3000 + 999 ) / 1000 = 3

@egernst
Copy link
Member Author

egernst commented Jun 11, 2019

@jcvenegas overall, though, the documentation is wrong.

It should be vCPU = ceiling( quota / period )

Can you send a PR to update the doc? WDYT?

jcvenegas added a commit to jcvenegas/documentation that referenced this issue Jun 11, 2019
The formula is not updated according on
how is done in kata-runtime.

Fixes: kata-containers#489

Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Incorrect behaviour needs-review Needs to be assessed by the team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants