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

Enable cpu limiting using cgroup v2 #8471

Merged
merged 2 commits into from
Mar 8, 2022
Merged

Enable cpu limiting using cgroup v2 #8471

merged 2 commits into from
Mar 8, 2022

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Feb 27, 2022

Description

Enable cpu limiting using cgroup v2.

Related Issue(s)

#8498

How to test

  • Open workspace in preview
  • Observe cgroup values being updated

Notes for reviewers

Considered using containerd cgroups library but

  • The api for v1 and v2 is different so we still would have duplication.
  • Updating a cgroup is currently not supported.

Release Notes

Support cpu limiting using cgroup v2

@codecov
Copy link

codecov bot commented Feb 27, 2022

Codecov Report

Merging #8471 (abae1db) into main (7ac5d9d) will increase coverage by 16.61%.
The diff coverage is 14.36%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8471       +/-   ##
===========================================
+ Coverage   12.31%   28.93%   +16.61%     
===========================================
  Files          20      150      +130     
  Lines        1161    23097    +21936     
===========================================
+ Hits          143     6683     +6540     
- Misses       1014    15799    +14785     
- Partials        4      615      +611     
Flag Coverage Δ
components-blobserve-app 31.34% <ø> (?)
components-blobserve-lib 31.34% <ø> (?)
components-common-go-lib 35.47% <29.41%> (?)
components-content-service-api-go-lib ∅ <ø> (?)
components-content-service-app 14.20% <ø> (?)
components-content-service-lib 14.20% <ø> (?)
components-ee-agent-smith-app 40.03% <ø> (?)
components-ee-agent-smith-lib 40.03% <ø> (?)
components-ee-kedge-app 45.48% <ø> (?)
components-gitpod-cli-app 11.17% <ø> (ø)
components-ide-code-codehelper-app ∅ <ø> (?)
components-image-builder-api-go-lib ∅ <ø> (?)
components-image-builder-bob-app ∅ <ø> (?)
components-image-builder-bob-runc-facade ∅ <ø> (?)
components-image-builder-mk3-app 35.71% <ø> (?)
components-installation-telemetry-app ∅ <ø> (?)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
components-openvsx-proxy-app 45.86% <ø> (?)
components-openvsx-proxy-lib 45.86% <ø> (?)
components-registry-facade-app 11.40% <ø> (?)
components-registry-facade-lib 11.40% <ø> (?)
components-service-waiter-app ∅ <ø> (?)
components-supervisor-app 35.37% <ø> (?)
components-workspacekit-app 7.01% <ø> (?)
components-ws-daemon-api-go-lib ∅ <ø> (?)
components-ws-daemon-app 20.40% <4.71%> (?)
components-ws-daemon-lib 20.40% <4.71%> (?)
components-ws-daemon-nsinsider-app ∅ <ø> (?)
components-ws-manager-api-go-lib ∅ <ø> (?)
components-ws-manager-app 39.44% <ø> (?)
components-ws-proxy-app 68.63% <ø> (?)
dev-loadgen-app ∅ <ø> (?)
dev-poolkeeper-app ∅ <ø> (?)
install-installer-raw-app 4.49% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/ws-daemon/pkg/cpulimit/cfs_v2.go 0.00% <0.00%> (ø)
components/ws-daemon/pkg/cpulimit/cpulimit.go 68.83% <ø> (ø)
components/ws-daemon/pkg/cpulimit/dispatch.go 0.00% <0.00%> (ø)
components/common-go/cgroups/cgroup.go 29.41% <29.41%> (ø)
components/ws-daemon/pkg/cpulimit/cfs.go 30.52% <55.55%> (ø)
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go
components/content-service/pkg/archive/tar.go 60.31% <0.00%> (ø)
components/content-service/pkg/initializer/git.go 0.00% <0.00%> (ø)
components/supervisor/pkg/dropwriter/dropwriter.go 73.46% <0.00%> (ø)
... and 127 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ac5d9d...abae1db. Read the comment docs.

@Furisto Furisto force-pushed the fo/v2cpulimit branch 2 times, most recently from 1f532ba to 1c78a42 Compare February 28, 2022 14:45
@roboquat roboquat added size/XL and removed size/L labels Feb 28, 2022
@utam0k
Copy link
Contributor

utam0k commented Mar 1, 2022

@Furisto Nice work! I created the issue and update the description.
#8498

@roboquat roboquat added size/L and removed size/XL labels Mar 2, 2022
@Furisto Furisto force-pushed the fo/v2cpulimit branch 3 times, most recently from 9991c07 to 5c28118 Compare March 3, 2022 17:21
@Furisto Furisto marked this pull request as ready for review March 3, 2022 17:22
@Furisto Furisto requested a review from a team March 3, 2022 17:22
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Mar 3, 2022
Copy link
Contributor

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

I left some comments, but I like these codes!

components/ws-daemon/pkg/cpulimit/cfs_v2.go Show resolved Hide resolved
components/ws-daemon/pkg/cpulimit/cfs_v2.go Show resolved Hide resolved
return 0, 0, xerrors.Errorf("cpu.max did not have expected number of fields: %s", parts)
}

var quota int64
Copy link
Contributor

Choose a reason for hiding this comment

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

I think quota is always a positive value, right?

Suggested change
var quota int64
var quota uint64

components/ws-daemon/pkg/cpulimit/dispatch.go Outdated Show resolved Hide resolved
@utam0k
Copy link
Contributor

utam0k commented Mar 4, 2022

This PR is quite a great feature for users. I would love to see you write a release note.

@Furisto
Copy link
Member Author

Furisto commented Mar 4, 2022

This PR is quite a great feature for users. I would love to see you write a release note.

Wasn't sure if we already want to announce it to users 😉 . Added release note.

@Furisto
Copy link
Member Author

Furisto commented Mar 7, 2022

/werft run

👍 started the job as gitpod-build-fo-v2cpulimit.13

@Furisto
Copy link
Member Author

Furisto commented Mar 7, 2022

@utam0k @csweichel PTAL

return Unknown, err
}

func IsUnifiedCgroupSetup() (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for reflecting my opinion❤️

Copy link
Contributor

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

I like these codes! LGTM

@roboquat roboquat merged commit 409fa71 into main Mar 8, 2022
@roboquat roboquat deleted the fo/v2cpulimit branch March 8, 2022 00:14
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/XL team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants