Enable large alloc warnings above 128KiB by default #16828
Conversation
We already have the reactor stall warnings on in "prod" and arguably the large allocs are more important as they actually might result in a crash. Further the large alloc threshold automatically increases after each warning so it's unlikely for this to be very spamy.
Remove the passing of the large alloc warning threshold in DT. We now set it to 128KiB by default in the binary so we don't want to override it 256KiB in DT. Note this doesn't affect the actual BadLogLines check as that has a separate limit of 400KiB for marking log lines as hard failures.
StephanDollberg
force-pushed
the
stephan/enable-large-alloc-warning-by-default
branch
from
bd37c88
to
a9003cc
Compare
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45545#018dfa58-a9fc-4451-aea7-1996a90193d2 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45545#018dfa58-a9f1-418a-b975-30b8d0931666 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45545#018dfa58-a9f8-4dff-b225-1215403d6ed0 |
| @@ -158,7 +158,7 @@ node_config::node_config() noexcept | |||
| "memory_allocation_warning_threshold", | |||
| "Enables log messages for allocations greater than the given size.", | |||
| {.visibility = visibility::tunable}, | |||
| std::nullopt) | |||
| 128_KiB + 1) // 128 KiB is the largest allowed allocation size | |||
There was a problem hiding this comment.
I'm not sure the warning triggers > or >= the threshold but I guess this works OK either way since acutal 128K + 1 allocations would seem to be extremely rare.
| @@ -3432,12 +3432,6 @@ def write_node_conf_file(self, | |||
| except: | |||
| cur_ver = None | |||
|
|
|||
| # This node property isn't available on versions of RP older than 23.2. | |||
| if cur_ver and cur_ver >= (23, 2, 0): | |||
| memory_allocation_warning_threshold_bytes = 256 * 1024 # 256 KiB | |||
There was a problem hiding this comment.
stephan or @ballard26 do you know why this was set to 256K in the first place?
There was a problem hiding this comment.
I was originally scraping all the test results to find oversized allocations. And to avoid too much log spam I set it to 512K initially, then lowered it to 256K, with the eventual goal of lowering it to 128K once the larger oversized allocations were fixed.
|
/backport v23.3.x |
We already have the reactor stall warnings on in "prod" and arguably the
large allocs are more important as they actually might result in a
crash.
Further the large alloc threshold automatically increases after each
warning so it's unlikely for this to be very spamy.
Backports Required
Release Notes
Improvements