Skip to content

[#26065] Add cgroup v2 support#34883

Merged
mlopatkin merged 2 commits into
gradle:masterfrom
Megmeehey:26065
Oct 29, 2025
Merged

[#26065] Add cgroup v2 support#34883
mlopatkin merged 2 commits into
gradle:masterfrom
Megmeehey:26065

Conversation

@Megmeehey

@Megmeehey Megmeehey commented Sep 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #26065

Context

Gradle currently only respects cgroup v1 memory limits and ignores cgroup v2.
As a result, when running in a k8s pod on a cgroup v2 host, Gradle can not detect available memory and limits, therefore it keeps idle processes alive, assuming it owns all node memory, and that leads to pod being killed by OOM killer.
This PR adds cgroup v2 support: Gradle now reads /sys/fs/cgroup/memory.current and /sys/fs/cgroup/memory.max, same way it did with v1.

Related doc: https://docs.kernel.org/admin-guide/cgroup-v2.html

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes.
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@Megmeehey Megmeehey requested a review from a team as a code owner September 3, 2025 16:25
@Megmeehey Megmeehey requested review from bamboo and mlopatkin and removed request for a team September 3, 2025 16:25
@bot-gradle bot-gradle added from:contributor PR by an external contributor to-triage labels Sep 3, 2025
@Megmeehey
Add cgroup v2 support
1d26bf4 
Signed-off-by: Megmeehey <megmeehey@gmail.com>
@ov7a

ov7a commented Sep 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Thank you for your proposed contribution!

This PR has a valid DCO. The relevant team for this area will confirm the design of the implementation choices.

Note that while it would be nice to cover the new code with tests somehow, apparently, we don't have good coverage for the existing code. For example, we don't test accessing specific file paths. The team will decide if it's a blocker.

@Megmeehey, meanwhile, can you look into making an integration test (maybe with mocks for system files)?

FTR, a PR that introduced cgroups v1:

@ov7a ov7a added in:core-utilities base services 👋 team-triage Issues that need to be triaged by a specific team and removed to-triage labels Sep 4, 2025
@Megmeehey

Megmeehey commented Sep 4, 2025

Copy link
Copy Markdown
Contributor Author

@ov7a

Hmm... From my perspective an integration test here would have limited value. The cgroup memory files only expose a single number, so there’s no complex parsing to validate.
The only thing a new test could cover is the logic checking whether the files exist at the expected paths, which isn’t something I can test with real pseudo files.

A feasible way to add a test I see would be to introduce another constructor that accepts file paths for cgroup v1 and v2.
Then I can pass temporary files to it... but that feels like too much faking for a little gain.

If there is a need for such a test, here is what I come up with: 1dcf88c
Please, see if that fits. If so, I can add it to this PR.

@ov7a

ov7a commented Sep 4, 2025

Copy link
Copy Markdown
Member

@Megmeehey Good points, thanks for looking into that.
I don't have a strong opinion on adding tests from that commit (however, I think it improves things). I'll delegate this decision to the team responsible for that area.

@mlopatkin mlopatkin linked an issue Sep 7, 2025 that may be closed by this pull request
When --no-daemons Is Set Parent Process Should Wait for Completion #34854
Open
@mlopatkin mlopatkin removed a link to an issue Sep 7, 2025
When --no-daemons Is Set Parent Process Should Wait for Completion #34854
Open
@bamboo bamboo added this to the 9.3.0 RC1 milestone Sep 11, 2025
@bamboo bamboo removed the 👋 team-triage Issues that need to be triaged by a specific team label Sep 11, 2025
@bamboo

bamboo commented Sep 11, 2025

Copy link
Copy Markdown
Member

We're considering this feature for Gradle 9.3 and will get back to you in a few weeks.

@mlopatkin mlopatkin self-assigned this Sep 19, 2025
@trourance

trourance commented Oct 7, 2025

Copy link
Copy Markdown

We're considering this feature for Gradle 9.3 and will get back to you in a few weeks.

Any chance to see this back-ported ?

mlopatkin
mlopatkin approved these changes Oct 23, 2025
View reviewed changes

@mlopatkin mlopatkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My understanding of cgroups is rather shallow, but it looks like we'll serve only a limited number of use cases here (no child groups, no custom mount points). It should be enough for a typical container where the limit is applied to the whole container. Nevertheless, it is as complete as our v1 support is today.

@mlopatkin

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@mlopatkin

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@bot-gradle

bot-gradle commented Oct 24, 2025

Copy link
Copy Markdown
Collaborator

The following builds have passed:

@mlopatkin mlopatkin added this pull request to the merge queue Oct 29, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 29, 2025
@mlopatkin

mlopatkin commented Oct 29, 2025

Copy link
Copy Markdown
Member

@bot-gradle test this

@bot-gradle

This comment has been minimized.

Sign in to view
@bot-gradle

bot-gradle commented Oct 29, 2025

Copy link
Copy Markdown
Collaborator

The following builds have failed:

@mlopatkin mlopatkin added this pull request to the merge queue Oct 29, 2025
@bot-gradle

bot-gradle commented Oct 29, 2025

Copy link
Copy Markdown
Collaborator

WARN: Based on labels, this pull request addresses notable issue but no changes to release note found.

Merged via the queue into gradle:master with commit 2184ef9 Oct 29, 2025
25 of 29 checks passed
@bric3

bric3 commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

This would be nice if this was backported to Gradle 8.14.

@bric3 bric3 mentioned this pull request Dec 18, 2025
Open
@ljacomet ljacomet mentioned this pull request Jan 22, 2026
Closed
mlopatkin added a commit that referenced this pull request Jan 22, 2026
@mlopatkin
[#26065] Add cgroup v2 support (#34883)
c458e6b 
(cherry picked from commit 2184ef9)
@cobexer cobexer mentioned this pull request Mar 18, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mlopatkin mlopatkin mlopatkin approved these changes

@bamboo bamboo Awaiting requested review from bamboo bamboo is a code owner automatically assigned from gradle/bt-cortex

Assignees

@mlopatkin mlopatkin

Labels

from:contributor PR by an external contributor in:core-utilities base services

Projects

None yet

Milestone

9.3.0 RC1

Development

Successfully merging this pull request may close these issues.

Support for cgroups v2

7 participants

@Megmeehey @ov7a @bamboo @trourance @mlopatkin @bot-gradle @bric3