-
Notifications
You must be signed in to change notification settings - Fork 666
datalake: dlq corrupted records #27327
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
base: dev
Are you sure you want to change the base?
datalake: dlq corrupted records #27327
Conversation
6c68128
to
0d6313c
Compare
@@ -609,4 +639,42 @@ record_multiplexer::handle_invalid_record( | |||
co_return std::nullopt; | |||
} | |||
} | |||
|
|||
ss::future<result<void, writer_error>> | |||
record_multiplexer::handle_corrupted_record( |
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.
IMO this would be less surprising for callers / more accurate if it were called handle_corrupted_batch()
, given we're treating the rest of the batch as corrupted, not just one record
_log.warn, | ||
"Error reading record from batch: {} at index {}. Err: {}", | ||
batch.header(), |
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.
nit: probably warrants an "error" log given the rarity of this situation.
translation_probe::invalid_record_cause::corrupted_record, | ||
o, | ||
std::nullopt, | ||
data_copy.share(0, data_copy.size_bytes()), |
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.
I'm wondering if we can only have the data payload for the first offset..
In the particular case where this crashed, the batch was > 100 MiB, seems wasteful to copy it over so many times.
ref https://redpandadata.atlassian.net/browse/INC-923
Backports Required
Release Notes
Bug Fixes