From 4ce318bf41b2da43b044e5ad536a20a3c3102949 Mon Sep 17 00:00:00 2001 From: Daniel Sampliner Date: Thu, 30 Jan 2025 15:34:40 -0500 Subject: [PATCH 1/7] fix: parallelism CPU calculation inside k8s The value of /sys/fs/cgroup/cpu/cpu.cfs_quota_us is not in milliseconds and cannot be simply divided by 1000 to determine the CPU limit. As per kernel documentation[^1], the cpu limit can be determined by dividing that value by /sys/fs/cgroup/cpu/cpu.cfs_period_us. [^1]: https://docs.kernel.org/scheduler/sched-bwc.html --- hooks/_common.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index fc0c930a7..801692bc8 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -197,14 +197,15 @@ function common::is_hook_run_on_whole_repo { function common::get_cpu_num { local -r parallelism_ci_cpu_cores=$1 + local cpu_quota cpu_period cpu_num local millicpu if [[ -f /sys/fs/cgroup/cpu/cpu.cfs_quota_us && ! -f /proc/sys/fs/binfmt_misc/WSLInterop ]]; then # WSL have cfs_quota_us, but WSL should be checked as usual Linux host # Inside K8s pod or DinD in K8s - millicpu=$(< /sys/fs/cgroup/cpu/cpu.cfs_quota_us) + cpu_quota=$(< /sys/fs/cgroup/cpu/cpu.cfs_quota_us) - if [[ $millicpu -eq -1 ]]; then + if [[ $cpu_quota -eq -1 ]]; then # K8s no limits or in DinD if [[ -n $parallelism_ci_cpu_cores ]]; then if [[ ! $parallelism_ci_cpu_cores =~ ^[[:digit:]]+$ ]]; then @@ -233,7 +234,13 @@ function common::get_cpu_num { return fi - echo $((millicpu / 1000)) + cpu_period=$(< /sys/fs/cgroup/cpu/cpu.cfs_period_us) + cpu_num=$((cpu_quota / cpu_period)) + if ((cpu_num < 1)); then + echo 1 + else + echo $cpu_num + fi return fi From 6af478f89ee6c959b5342860cf3c092d87db55b2 Mon Sep 17 00:00:00 2001 From: Daniel Sampliner Date: Thu, 30 Jan 2025 21:10:25 -0500 Subject: [PATCH 2/7] refactor: style Co-authored-by: George L. Yermulnik --- hooks/_common.sh | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index 801692bc8..801123000 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -236,11 +236,7 @@ function common::get_cpu_num { cpu_period=$(< /sys/fs/cgroup/cpu/cpu.cfs_period_us) cpu_num=$((cpu_quota / cpu_period)) - if ((cpu_num < 1)); then - echo 1 - else - echo $cpu_num - fi + [[ $cpu_num -lt 1 ]] && echo 1 || echo $cpu_num return fi From b426613a6376c1d670919457c9b889355008ba8d Mon Sep 17 00:00:00 2001 From: Daniel Sampliner Date: Thu, 30 Jan 2025 21:16:56 -0500 Subject: [PATCH 3/7] fix: clean error message when unexpected cpu.cfs_quota_us --- hooks/_common.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index 801123000..edc1ddd1b 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -204,8 +204,9 @@ function common::get_cpu_num { ! -f /proc/sys/fs/binfmt_misc/WSLInterop ]]; then # WSL have cfs_quota_us, but WSL should be checked as usual Linux host # Inside K8s pod or DinD in K8s cpu_quota=$(< /sys/fs/cgroup/cpu/cpu.cfs_quota_us) + cpu_period=$(cat /sys/fs/cgroup/cpu/cpu.cfs_period_us || echo "$cpu_quota") - if [[ $cpu_quota -eq -1 ]]; then + if [[ $cpu_quota -eq -1 || $cpu_period -lt 1 ]]; then # K8s no limits or in DinD if [[ -n $parallelism_ci_cpu_cores ]]; then if [[ ! $parallelism_ci_cpu_cores =~ ^[[:digit:]]+$ ]]; then @@ -234,7 +235,6 @@ function common::get_cpu_num { return fi - cpu_period=$(< /sys/fs/cgroup/cpu/cpu.cfs_period_us) cpu_num=$((cpu_quota / cpu_period)) [[ $cpu_num -lt 1 ]] && echo 1 || echo $cpu_num return From 9bd7a26401e2148180865bac44cb9183134bc12c Mon Sep 17 00:00:00 2001 From: Daniel Sampliner Date: Fri, 31 Jan 2025 12:39:39 -0500 Subject: [PATCH 4/7] fix: suppress unnecessary error Co-authored-by: George L. Yermulnik --- hooks/_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index edc1ddd1b..bfc3b432e 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -204,7 +204,7 @@ function common::get_cpu_num { ! -f /proc/sys/fs/binfmt_misc/WSLInterop ]]; then # WSL have cfs_quota_us, but WSL should be checked as usual Linux host # Inside K8s pod or DinD in K8s cpu_quota=$(< /sys/fs/cgroup/cpu/cpu.cfs_quota_us) - cpu_period=$(cat /sys/fs/cgroup/cpu/cpu.cfs_period_us || echo "$cpu_quota") + cpu_period=$(cat /sys/fs/cgroup/cpu/cpu.cfs_period_us 2> /dev/null || echo "$cpu_quota") if [[ $cpu_quota -eq -1 || $cpu_period -lt 1 ]]; then # K8s no limits or in DinD From 4a77613df549311bc78ace238da50b6c6274c1b9 Mon Sep 17 00:00:00 2001 From: MaxymVlasov Date: Sat, 1 Feb 2025 00:12:02 +0200 Subject: [PATCH 5/7] chore: Add docs and move vars inside their if's --- hooks/_common.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index bfc3b432e..4ffff3cea 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -197,12 +197,12 @@ function common::is_hook_run_on_whole_repo { function common::get_cpu_num { local -r parallelism_ci_cpu_cores=$1 - local cpu_quota cpu_period cpu_num - local millicpu - if [[ -f /sys/fs/cgroup/cpu/cpu.cfs_quota_us && ! -f /proc/sys/fs/binfmt_misc/WSLInterop ]]; then # WSL have cfs_quota_us, but WSL should be checked as usual Linux host # Inside K8s pod or DinD in K8s + local cpu_quota cpu_period cpu_num + # CPU quota should be calculated as `cpu.cfs_quota_us / cpu.cfs_period_us` + # See: https://docs.kernel.org/scheduler/sched-bwc.html cpu_quota=$(< /sys/fs/cgroup/cpu/cpu.cfs_quota_us) cpu_period=$(cat /sys/fs/cgroup/cpu/cpu.cfs_period_us 2> /dev/null || echo "$cpu_quota") @@ -242,6 +242,7 @@ function common::get_cpu_num { if [[ -f /sys/fs/cgroup/cpu.max ]]; then # Inside Linux (Docker?) container + local millicpu millicpu=$(cut -d' ' -f1 /sys/fs/cgroup/cpu.max) if [[ $millicpu == max ]]; then From 70eefc825c00760c3a07b481b391f4f864c37309 Mon Sep 17 00:00:00 2001 From: MaxymVlasov Date: Sat, 1 Feb 2025 00:33:55 +0200 Subject: [PATCH 6/7] fix: Docker CPU limits --- hooks/_common.sh | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index 4ffff3cea..0a1c62d83 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -188,6 +188,11 @@ function common::is_hook_run_on_whole_repo { ####################################################################### # Get the number of CPU logical cores available for pre-commit to use + +# CPU quota should be calculated as `cpu.cfs_quota_us / cpu.cfs_period_us` +# For K8s see: https://docs.kernel.org/scheduler/sched-bwc.html +# For Docker see: https://docs.docker.com/engine/containers/resource_constraints/#configure-the-default-cfs-scheduler +# # Arguments: # parallelism_ci_cpu_cores (string) Used in edge cases when number of # CPU cores can't be derived automatically @@ -197,12 +202,11 @@ function common::is_hook_run_on_whole_repo { function common::get_cpu_num { local -r parallelism_ci_cpu_cores=$1 + local cpu_quota cpu_period cpu_num + if [[ -f /sys/fs/cgroup/cpu/cpu.cfs_quota_us && ! -f /proc/sys/fs/binfmt_misc/WSLInterop ]]; then # WSL have cfs_quota_us, but WSL should be checked as usual Linux host # Inside K8s pod or DinD in K8s - local cpu_quota cpu_period cpu_num - # CPU quota should be calculated as `cpu.cfs_quota_us / cpu.cfs_period_us` - # See: https://docs.kernel.org/scheduler/sched-bwc.html cpu_quota=$(< /sys/fs/cgroup/cpu/cpu.cfs_quota_us) cpu_period=$(cat /sys/fs/cgroup/cpu/cpu.cfs_period_us 2> /dev/null || echo "$cpu_quota") @@ -242,16 +246,17 @@ function common::get_cpu_num { if [[ -f /sys/fs/cgroup/cpu.max ]]; then # Inside Linux (Docker?) container - local millicpu - millicpu=$(cut -d' ' -f1 /sys/fs/cgroup/cpu.max) + cpu_quota=$(cut -d' ' -f1 /sys/fs/cgroup/cpu.max) + cpu_period=$(cut -d' ' -f2 /sys/fs/cgroup/cpu.max) - if [[ $millicpu == max ]]; then + if [[ $cpu_quota == max || $cpu_period -lt 1 ]]; then # No limits nproc 2> /dev/null || echo 1 return fi - echo $((millicpu / 1000)) + cpu_num=$((cpu_quota / cpu_period)) + [[ $cpu_num -lt 1 ]] && echo 1 || echo $cpu_num return fi From 97d7f3ac8aced518969022315d856c6e654f92c0 Mon Sep 17 00:00:00 2001 From: Maksym Vlasov Date: Sat, 1 Feb 2025 00:36:55 +0200 Subject: [PATCH 7/7] Apply suggestions from code review --- hooks/_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/_common.sh b/hooks/_common.sh index 0a1c62d83..e9defff52 100644 --- a/hooks/_common.sh +++ b/hooks/_common.sh @@ -188,7 +188,7 @@ function common::is_hook_run_on_whole_repo { ####################################################################### # Get the number of CPU logical cores available for pre-commit to use - +# # CPU quota should be calculated as `cpu.cfs_quota_us / cpu.cfs_period_us` # For K8s see: https://docs.kernel.org/scheduler/sched-bwc.html # For Docker see: https://docs.docker.com/engine/containers/resource_constraints/#configure-the-default-cfs-scheduler