CORE-1614 storage: Initialize timestamps in the compaction_placeholder #17716
Conversation
compaction_placeholder batch. The batch wasn't initialized properly and because of that the invalid timestamps leaked into the TS layer.
| @@ -135,6 +135,8 @@ model::record_batch copy_data_segment_reducer::make_placeholder_batch( | |||
| new_hdr.type = model::record_batch_type::compaction_placeholder; | |||
| new_hdr.base_offset = hdr.base_offset; | |||
| new_hdr.last_offset_delta = hdr.last_offset_delta; | |||
| new_hdr.first_timestamp = hdr.first_timestamp; | |||
There was a problem hiding this comment.
storage: Initialize timestamps in the ...
nit: I guess the contributing guide mentions line length, but is this style really preferred over a line over 50 characters? I've been assuming 50 was just a suggestion to generally keep the titles short, but this seems harder to grok over git history than a slightly longer line
There was a problem hiding this comment.
the title of the PR is different from the title of the commit (which has proper size)
There was a problem hiding this comment.
Initialize timestamps in the ...
The subject of a commit message, in this case storage: Initialize timestamps in the ... is the thing that shows up when you do a git shortlog. It's not really intended to be the first part of the commit message body, it's generally useful to have that be understandable without the context of the body of the commit message. In this case that isn't true.
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/47577#018ec452-5545-4aaf-9b88-32441bdc7bb4 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/47577#018ec6a0-a6d0-4019-8361-438032c90318 |
dotnwat
changed the title
|
CI failures are caused by Maven build. |
The
compaction_placeholderbatch wasn't initialized properly and because of that the invalid timestamps equal to -1 leaked into the TS layer. Therp-storage-toolcrashed while trying to parse the list of timestamps.Fixes #15312
Backports Required
Release Notes