Add oversized allocation to bad log lines #16083
Conversation
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43700#018cfb76-3b4f-4169-8836-195030ce78c5 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43700#018cfb7c-160a-42bd-9754-1d347c903ed7 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43730#018cff08-ef8d-48c9-9d08-5c1c6a77904f |
tests/rptest/services/redpanda.py
Outdated
| f"Ignoring oversized allocation, {m.group(1)} is less than the max allowable allocation size of {MAX_ALLOCATION_SIZE} bytes" | ||
| ) | ||
| allowed = True | ||
| break |
There was a problem hiding this comment.
This needs to be a continue otherwise we ignore other potential log lines? Seems broken in the leak sanitizer check as well.
There was a problem hiding this comment.
LeakSanitizer looks like it's doing the right thing: the break is in a nested for which is re-grepping the entire log (!!) to see if the leak is small enough to ignored, so it should just result in skipping to the next line.
The break here def looks like it would silently skip the rest of the lines though, good eye!
There was a problem hiding this comment.
Nice catch, fixing now
| @@ -1317,6 +1321,15 @@ def raise_on_bad_logs(self, allow_list=None): | |||
| allowed = True | |||
| break | |||
|
|
|||
| if "oversized allocation" in line: | |||
| m = re.search("oversized allocation: (\d+) byte", line) | |||
There was a problem hiding this comment.
Just checking because I always struggle with regexes. It not saying "bytes" at the end there is fine? This is a simple match?
There was a problem hiding this comment.
search() is the one that looks for a match anywhere in the line, i.e., with arbitrary leading and trailing unmatched characters so this would match ... bytes.
| @@ -186,6 +186,9 @@ | |||
| re.compile("finject - .* flush called concurrently with other operations") | |||
| ] | |||
|
|
|||
| # Largest allocation allowed in during a test | |||
There was a problem hiding this comment.
nit: the largest allocation allowed is 1 less than this value since we use < with this on the rhs
There was a problem hiding this comment.
Nice find, making it the actual largest alloc allowable.
|
@ballard26 looks good other the the Were you able to test this in any way? Maybe a dummy PR with the max value set to 64k or something since that should definitely turn up some failures. |
ballard26
force-pushed
the
oversized-failure
branch
from
5f33450
to
f7948d8
Compare
I've tested it locally with two different admin API endpoints. One that makes and allocation of 300KiB and one that makes an allocation of 500KiB. It works as expected allowing for 300KiB, but leaving a warning in the log, and failing the test for 500KiB. |
|
This was all discussed and feedback addressed per @ballard26 ; merging. |
This will cause a ducktape test to fail if an allocation of 400KiB or more occurs during it.
Backports Required
Release Notes