Skip to content

Commit 00062db

Browse files
Aangbaeckclaude
andcommitted
storage: fix compaction minting future record timestamps
copy_data_segment_reducer::filter() re-encodes a batch when some of its records are compacted away. It advanced the new batch's first_timestamp to the first surviving record's absolute timestamp, but did NOT rebase the per-record timestamp deltas, which remain relative to the batch's original first_timestamp. On read, every surviving record therefore resolves to new_first_timestamp + delta, i.e. shifted forward by a constant (the first surviving record's original delta). This is only non-zero when the batch's base record (delta 0) is dropped (e.g. a tombstone), and the shift is large and positive when the batch is out of order (the dropped base record is much older than the first surviving record) -- turning strictly-past records into records with future timestamps. This breaks broker timestamp validation (log.message.timestamp.after.max.ms) and freezes downstream Kafka Streams monotonic-max KTable aggregates in the future. Replicas compact independently, so the same offset can carry different timestamps on different brokers. Offsets are unaffected because base_offset is preserved on compaction; this restores the same symmetry for timestamps. Fix: preserve the original first_timestamp (so the unchanged per-record deltas stay valid) and compute max_timestamp from the greatest surviving delta (out-of-order safe), instead of assuming the last record is the max. Reproduced on v26.1.6 and v26.1.12 (compact topic + out-of-order timestamps + tombstones): records produced strictly in the past read back with timestamps hundreds of days in the future after compaction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Aangbaeck <3142272+Aangbaeck@users.noreply.github.com>
1 parent 6248e7f commit 00062db

1 file changed

Lines changed: 22 additions & 10 deletions

File tree

src/v/storage/compaction_reducers.cc

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,16 +230,19 @@ copy_data_segment_reducer::filter(model::record_batch batch) {
230230
// 4. filter
231231
iobuf ret;
232232
int32_t rec_count = 0;
233-
std::optional<int64_t> first_timestamp_delta;
234-
int64_t last_timestamp_delta;
233+
// Track the MAX surviving timestamp delta. Records may be out of order, so the last
234+
// surviving record is not necessarily the one with the greatest timestamp. The per-record
235+
// deltas are appended UNCHANGED (relative to the batch's original first_timestamp, which is
236+
// preserved below) -- mirroring how base_offset is preserved so that offset deltas stay
237+
// valid across compaction.
238+
std::optional<int64_t> max_timestamp_delta;
235239
// We expect and enforce that offset_deltas is sorted.
236240
dassert(
237241
std::ranges::is_sorted(offset_deltas),
238242
"offset_deltas must be ascending in record-iteration order");
239243
size_t keep_idx = 0;
240244
batch.for_each_record([&rec_count,
241-
&first_timestamp_delta,
242-
&last_timestamp_delta,
245+
&max_timestamp_delta,
243246
&ret,
244247
&keep_idx,
245248
&offset_deltas](model::record record) {
@@ -255,10 +258,11 @@ copy_data_segment_reducer::filter(model::record_batch batch) {
255258
* record batch with the uncompressed records so they were being
256259
* re-encoded.
257260
*/
258-
if (!first_timestamp_delta) {
259-
first_timestamp_delta = record.timestamp_delta();
261+
if (
262+
!max_timestamp_delta
263+
|| record.timestamp_delta() > *max_timestamp_delta) {
264+
max_timestamp_delta = record.timestamp_delta();
260265
}
261-
last_timestamp_delta = record.timestamp_delta();
262266
model::append_record_to_buffer(ret, record);
263267
++rec_count;
264268
}
@@ -303,11 +307,19 @@ copy_data_segment_reducer::filter(model::record_batch batch) {
303307
// previous value prior to becoming empty.
304308
//
305309
auto& hdr = batch.header();
306-
const auto first_time = model::timestamp(
307-
hdr.first_timestamp() + first_timestamp_delta.value());
310+
// BUGFIX (future-timestamp mint via compaction): PRESERVE the original first_timestamp.
311+
// The surviving records above kept their original timestamp deltas (relative to this
312+
// first_timestamp), so their absolute timestamps remain correct. Previously first_timestamp
313+
// was advanced to the first surviving record's absolute timestamp WITHOUT rebasing the
314+
// per-record deltas -- which shifted every surviving record forward by first_timestamp_delta.
315+
// When the batch's original base record was dropped (e.g. a tombstone) and the batch was out
316+
// of order, that shift was large and positive, turning strictly-past records into records
317+
// with FUTURE timestamps. Preserving first_timestamp mirrors how base_offset is preserved.
318+
const auto first_time = hdr.first_timestamp;
308319
auto last_time = hdr.max_timestamp;
309320
if (hdr.attrs.timestamp_type() == model::timestamp_type::create_time) {
310-
last_time = model::timestamp(first_time() + last_timestamp_delta);
321+
last_time = model::timestamp(
322+
hdr.first_timestamp() + max_timestamp_delta.value());
311323
}
312324
auto new_hdr = hdr;
313325

0 commit comments

Comments
 (0)