-
Notifications
You must be signed in to change notification settings - Fork 590
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
fix(memory): increase reserved mem to 30% #13648
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #13648 +/- ##
==========================================
- Coverage 68.18% 68.18% -0.01%
==========================================
Files 1521 1521
Lines 261709 261709
==========================================
- Hits 178454 178442 -12
- Misses 83255 83267 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b640bc9
to
c71a926
Compare
Previously I also made similar adjustments, hoping to pass the longevity test.
and the longevity test passed without limit rdkakfa fetch_queue_size Not sure whether should we reduce storage memory size in the meantime. cc @hzxa21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Should we consider using |
PS. At the beginning, I planned to replace the |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
The
reserved_memory
is designed for the difference between actual usage (jemalloc_allocated
) and process usage (process_resident_memory
).risingwave/src/compute/src/memory/config.rs
Lines 40 to 42 in b640bc9
As being observed in #13610,
The ratio is
8.59/12.3 = 0.698 ≈ 0.7
.Passed longevity test: https://buildkite.com/risingwave-test/longevity-test/builds/818 (Previously this would OOM for 2 times)
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.