Skip to content

linux: align CPU quota calculation with Rust's cgroup heuristics#4746

Merged
bnoordhuis merged 1 commit intolibuv:v1.xfrom
juanarbol:juan/cgroups-cpu
Apr 22, 2025
Merged

linux: align CPU quota calculation with Rust's cgroup heuristics#4746
bnoordhuis merged 1 commit intolibuv:v1.xfrom
juanarbol:juan/cgroups-cpu

Conversation

@juanarbol
Copy link
Copy Markdown
Member

Fixes: #4740

@juanarbol
Copy link
Copy Markdown
Member Author

@bnoordhuis here we got some entertainment.

cc// @ggatus-atlassian feel free to pull and test.

@bnoordhuis
Copy link
Copy Markdown
Member

I'll wait with reviewing until it's been tested.

@juanarbol
Copy link
Copy Markdown
Member Author

I've tested the thing. This is my setup:

Image for libuv-runner:

FROM debian:bookworm

RUN apt-get update && apt-get install -y \
  gdb ca-certificates bash

USER 1000:1000
WORKDIR /home/devuser
CMD ["/bin/bash", "-c", "sleep infinity"]

Kube pod manifest:

apiVersion: v1
kind: Pod
metadata:
  name: libuv-platform-test
spec:
  restartPolicy: Never
  securityContext:
    runAsUser: 1000
    runAsGroup: 1000
    fsGroup: 1000
  volumes:
    - name: build-volume
      emptyDir: {}

  initContainers:
    - name: builder
      image: debian:bookworm
      imagePullPolicy: Always
      securityContext:
        runAsUser: 0
      command:
        - /bin/bash
        - -c
        - |
          apt-get update && apt-get install -y \
            git build-essential cmake curl python3 ca-certificates gdb \
            && git clone https://github.com/libuv/libuv.git /home/devuser/libuv \
            && cd /home/devuser/libuv \
            && git fetch origin pull/4746/head:pr-4746 && git checkout pr-4746 \
            && mkdir build && cd build \
            && cmake -DCMAKE_BUILD_TYPE=Debug .. && cmake --build . -j$(nproc)
      volumeMounts:
        - name: build-volume
          mountPath: /home/devuser
      resources:
        limits:
          cpu: "4"
          memory: 2Gi
        requests:
          cpu: "0"
          memory: 2Gi

  containers:
    - name: runner # --- PLEASE NOTE THIS
      image: libuv-runner:latest
      imagePullPolicy: Never
      command: ["/bin/bash"]
      args: ["-c", "sleep infinity"]
      volumeMounts:
        - name: build-volume
          mountPath: /home/devuser
      resources:
        limits:
          cpu: "4"
          memory: 2Gi
        requests:
          cpu: "0" # --- PLEASE NOTE THIS
          memory: 2Gi

I used gdb to make sure the implementation was doing what it is meant to do. For now, just part of the test output is enough:

$ kubectl exec -it libuv-platform -c runner -- /bin/bash
I have no name!@libuv-platform:/home/devuser$ cd libuv/build/
I have no name!@libuv-platform:/home/devuser/libuv/build$ ./uv_run_tests_a platform_output platform_output
uv_get_process_title: ./uv_run_tests_a
uv_cwd: /home/devuser/libuv/build
uv_resident_set_memory: 1867776
uv_uptime: 5519.560000
uv_getrusage:
  user: 0 sec 0 microsec
  system: 0 sec 1776 microsec
  page faults: 0
  maximum resident set size: 1824
uv_available_parallelism: 4
cgroup v2 available parallelism: 4
...

The implementation is correct as It iterates over the whole file-hierarchy, instead of giving up at the first read operation.

@juanarbol juanarbol force-pushed the juan/cgroups-cpu branch 3 times, most recently from 0bc4e9d to 3ddacea Compare April 8, 2025 17:34
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Mostly LGTM but could maybe use one more look-over from another maintainer.

@juanarbol juanarbol force-pushed the juan/cgroups-cpu branch 3 times, most recently from 99334d6 to 36338b8 Compare April 16, 2025 17:34
Copy link
Copy Markdown
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

LGTM with a tiny suggestion. Good work Juan!

Fixes: libuv#4740
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
@bnoordhuis bnoordhuis merged commit 69d2dfe into libuv:v1.x Apr 22, 2025
28 checks passed
@alumni
Copy link
Copy Markdown

alumni commented Apr 24, 2025

Any chance this would make it in node 22 anytime soon?

Our CI is 40-60% slower on node 22 compared to node 20.

@bnoordhuis
Copy link
Copy Markdown
Member

Wrong place to ask, that's up to node.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

uv_available_parallelism returns different results between cgroupsv1 and cgroupsv2 when running in Kubernetes from 1.49.1 onwards

4 participants