Skip to content

fix: use the correct cgroup policy to work on most of the systems#2421

Merged
alepane21 merged 1 commit intomainfrom
ale/eng-8671-gomemlimit-configuration-issue-on-aws-eks
Dec 18, 2025
Merged

fix: use the correct cgroup policy to work on most of the systems#2421
alepane21 merged 1 commit intomainfrom
ale/eng-8671-gomemlimit-configuration-issue-on-aws-eks

Conversation

@alepane21
Copy link
Copy Markdown
Contributor

@alepane21 alepane21 commented Dec 18, 2025

We are using a policy that supports cgroup v2 with hybrid layout and cgroups v1.
But systems that only supports pure cgroup v2 will not be correctly working.
So I changed it to use the FromCgroup that is checking for V2, V2 Hybrid and also V1 systems.

Summary by CodeRabbit

  • Refactor
    • Updated memory limit detection mechanism during initialization. The system's logging behavior for memory configuration remains unchanged, ensuring consistent operational transparency.

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 18, 2025

Walkthrough

Both plan_generator.go and supervisor_instance.go swap the memory limit provider from FromCgroupHybrid to FromCgroup during GOMEMLIMIT auto-configuration. The logging behavior for success and failure cases remains unchanged.

Changes

Cohort / File(s) Summary
Memory limit provider update
router/cmd/plan_generator.go, router/core/supervisor_instance.go
Replaces FromCgroupHybrid with FromCgroup when auto-configuring GOMEMLIMIT during initialization. Success and failure logging behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10–15 minutes

  • Memory limit provider is a direct, homogeneous replacement across two files with no control flow or error handling changes
  • Verify that FromCgroup provides equivalent or improved behavior compared to FromCgroupHybrid in both environments (plan generation and supervisor initialization)

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: replacing the cgroup policy from hybrid to the correct one to improve system compatibility.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 18, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-dcb4c86b8b83f486bfab3eb042798db16582cb2c

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 18, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.55%. Comparing base (8980fa8) to head (f8c3905).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
router/cmd/plan_generator.go 0.00% 1 Missing ⚠️
router/core/supervisor_instance.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2421      +/-   ##
==========================================
+ Coverage   60.75%   61.55%   +0.79%     
==========================================
  Files         229      229              
  Lines       23818    23814       -4     
==========================================
+ Hits        14470    14658     +188     
+ Misses       8101     7919     -182     
+ Partials     1247     1237      -10     
Files with missing lines Coverage Δ
router/cmd/plan_generator.go 0.00% <0.00%> (ø)
router/core/supervisor_instance.go 0.00% <0.00%> (ø)

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alepane21 alepane21 marked this pull request as ready for review December 18, 2025 11:43
@alepane21 alepane21 merged commit e647984 into main Dec 18, 2025
37 checks passed
@alepane21 alepane21 deleted the ale/eng-8671-gomemlimit-configuration-issue-on-aws-eks branch December 18, 2025 13:17
dbinnersley added a commit to dbinnersley/cosmo that referenced this pull request Jan 5, 2026
…ueries (wundergraph#2411)

chore(release): Publish [skip ci]

 - wgc@0.102.5
 - @wundergraph/composition@0.49.0
 - controlplane@0.181.0
 - graphqlmetrics@0.37.0
 - @wundergraph/protographic@0.14.0
 - router@0.273.0
 - @wundergraph/cosmo-shared@0.42.24
 - studio@0.147.0

feat: upgrade NextJS (wundergraph#2410)

chore(release): Publish [skip ci]

 - studio@0.148.0

fix: resolve js-yaml vulnerabilities (wundergraph#2415)

feat: validate session cookie (wundergraph#2406)

Co-authored-by: Milinda Dias <83293842+SkArchon@users.noreply.github.com>
chore(release): Publish [skip ci]

 - wgc@0.102.6
 - controlplane@0.182.0
 - keycloak@0.11.1

fix: codecov carryforward paths are not detected (wundergraph#2412)

chore: bumping expr dependency (wundergraph#2419)

fix: make error accessible to custom modules (wundergraph#2420)

chore(release): Publish [skip ci]

 - router@0.273.1

fix: use the correct cgroup policy to work on most of the systems (wundergraph#2421)

chore(release): Publish [skip ci]

 - router@0.273.2

Add support for per message deflate to websocket connectinos

Merge branch 'main' into per-message-deflate
revert formatting
dbinnersley added a commit to dbinnersley/cosmo that referenced this pull request Jan 5, 2026
…ueries (wundergraph#2411)

chore(release): Publish [skip ci]

 - wgc@0.102.5
 - @wundergraph/composition@0.49.0
 - controlplane@0.181.0
 - graphqlmetrics@0.37.0
 - @wundergraph/protographic@0.14.0
 - router@0.273.0
 - @wundergraph/cosmo-shared@0.42.24
 - studio@0.147.0

feat: upgrade NextJS (wundergraph#2410)

chore(release): Publish [skip ci]

 - studio@0.148.0

fix: resolve js-yaml vulnerabilities (wundergraph#2415)

feat: validate session cookie (wundergraph#2406)

Co-authored-by: Milinda Dias <83293842+SkArchon@users.noreply.github.com>
chore(release): Publish [skip ci]

 - wgc@0.102.6
 - controlplane@0.182.0
 - keycloak@0.11.1

fix: codecov carryforward paths are not detected (wundergraph#2412)

chore: bumping expr dependency (wundergraph#2419)

fix: make error accessible to custom modules (wundergraph#2420)

chore(release): Publish [skip ci]

 - router@0.273.1

fix: use the correct cgroup policy to work on most of the systems (wundergraph#2421)

chore(release): Publish [skip ci]

 - router@0.273.2

Add support for per message deflate to websocket connectinos

Merge branch 'main' into per-message-deflate
revert formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants