-
Notifications
You must be signed in to change notification settings - Fork 587
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
kafka: fix fetching from partitions with negative indexes #23301
Conversation
Some of the inputs may be directly from user requests, avoid asserting when encountering such malformed requests. Particularly requests like contains() and find() don't need to assert on non existent keys.
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/54407#0191e705-e156-4169-8482-f282de7c9f6b ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/54407#0191e707-9f4c-4b45-9f6b-8ea61545bb2e |
/backport v24.2.x |
key); | ||
static bool valid_key(KeyT key) { | ||
// map keys must be positive. | ||
return key >= 0; |
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.
why use a generic type if it must be convertible to int?
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.
Template definition restricts it to integral types
template<std::integral KeyT, typename ValueT>
class contiguous_range_map {
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.
yeh i guess if we had a fixed integral type then we'd end up with various narrowing conversion warnings etc...
Since 24.2.x this was triggering an assert. Added a regression test.
Fixes #23296
Backports Required
Release Notes
Bug Fixes