Skip to content

Commit e3bf527

Browse files
committed
storage: deprecate bogus timestamp adjusting codepaths
This stuff is all deprecated since we have timestamp validation in the produce path, and is actually quite undesirable behavior now. The cluster property `storage_ignore_timestamps_in_future_sec` should be eventually deprecated after enough time has passed (at the same time, we should set the `available_policy` of `validated_batch_timestamps` from `new_clusters_only` to `always`).
1 parent 77eedf2 commit e3bf527

2 files changed

Lines changed: 195 additions & 80 deletions

File tree

src/v/storage/disk_log_impl.cc

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -398,30 +398,40 @@ disk_log_impl::time_based_gc_max_offset(gc_config cfg) const {
398398
: _segs.front()->index().retention_timestamp(retention_cfg),
399399
retention_cfg);
400400

401+
auto validated_timestamps = _feature_table.local().is_active(
402+
features::feature::validated_batch_timestamps);
401403
static constexpr auto const_threshold = 1min;
402404
auto bogus_threshold = model::timestamp(
403405
model::timestamp::now().value() + const_threshold / 1ms);
404406

405407
auto it = std::find_if(
406408
std::cbegin(_segs),
407409
std::cend(_segs),
408-
[this, time = cfg.eviction_time, bogus_threshold, retention_cfg](
409-
const ss::lw_shared_ptr<segment>& s) {
410+
[this,
411+
time = cfg.eviction_time,
412+
bogus_threshold,
413+
retention_cfg,
414+
validated_timestamps](const ss::lw_shared_ptr<segment>& s) {
410415
auto retention_ts = s->index().retention_timestamp(retention_cfg);
411416

412-
if (retention_ts > bogus_threshold) {
413-
// Warn on timestamps more than the "bogus" threshold in future
414-
// this should not fire for segments created after v23.3
415-
vlog(
416-
gclog.warn,
417-
"[{}] found segment with bogus retention timestamp: {} (base "
418-
"{}, max {}) - {}",
419-
config().ntp(),
420-
retention_ts,
421-
s->index().base_timestamp(),
422-
s->index().max_timestamp(),
423-
s->index().broker_timestamp(),
424-
s);
417+
if (!validated_timestamps) {
418+
// Deprecated code path for >= v25.3.1.
419+
if (retention_ts > bogus_threshold) {
420+
// Warn on timestamps more than the "bogus" threshold in
421+
// future this should not fire for segments created after
422+
// v23.3
423+
vlog(
424+
gclog.warn,
425+
"[{}] found segment with bogus retention timestamp: {} "
426+
"(base "
427+
"{}, max {}) - {}",
428+
config().ntp(),
429+
retention_ts,
430+
s->index().base_timestamp(),
431+
s->index().max_timestamp(),
432+
s->index().broker_timestamp(),
433+
s);
434+
}
425435
}
426436

427437
// first that is not going to be collected
@@ -1776,6 +1786,12 @@ ss::future<std::optional<model::offset>> disk_log_impl::do_gc(gc_config cfg) {
17761786
}
17771787

17781788
ss::future<> disk_log_impl::maybe_adjust_retention_timestamps() {
1789+
auto validated_timestamps = _feature_table.local().is_active(
1790+
features::feature::validated_batch_timestamps);
1791+
if (validated_timestamps) {
1792+
// Deprecated code path for >= v25.3.1.
1793+
co_return;
1794+
}
17791795
// Correct any timestamps too far in the future, meant to be called before
17801796
// calculating the retention offset for garbage collection.
17811797
// It's expected that this will be used only for segments pre v23.3,

0 commit comments

Comments
 (0)