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

Remove cpu limits from K8s pod spec builder #3338

Merged
merged 2 commits into from
Nov 16, 2022
Merged

Conversation

bentsherman
Copy link
Member

Closes #3329

Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@sonatype-lift
Copy link

sonatype-lift bot commented Nov 1, 2022

⚠️ 7 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

@bentsherman
Copy link
Member Author

Matches the behavior in AWS Batch / ECS / Docker (in Linux):

https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html#container_definitions

The number of cpu units the Amazon ECS container agent reserves for the container. On Linux, this parameter maps to CpuShares in the Create a container section of the Docker Remote API and the --cpu-shares option to docker run.

Linux containers share unallocated CPU units with other containers on the container instance with the same ratio as their allocated amount. For example, assume that you run a single-container task on a single-core instance type with 512 CPU units specified for that container, and that task is the only task running on the container instance. In this example, the container can use the full 1,024 CPU unit share at any given time. However, assume then that you launched another copy of the same task on that container instance. Each task is guaranteed a minimum of 512 CPU units when needed, and each container can float to higher CPU usage if the other container was not using it. However, if both tasks were 100% active all of the time, they are limited to 512 CPU units.

@pditommaso
Copy link
Member

Yes. It looks like K8s request correspondß to --cpu-shares and limit corresponds to --cpu-quota or (--cpus).

https://docs.docker.com/config/containers/resource_constraints/

This is interesting because we are using --cpus also in docker.

result << "--cpus ${String.format(Locale.ROOT, "%.1f", cpus)} "

Therefore this should change to --cpu-shares as well (but in separate PR)

@pditommaso pditommaso mentioned this pull request Nov 13, 2022
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Copy link
Member

@pditommaso pditommaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just removed the use of @CompileDynamic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not use cpus limit requirement with K8s executor
3 participants