Skip to content

Commit 8c3777b

Browse files
committed
model: make next_offset saturate rather than overflow
model::offset::max() is often used to indicate "no upper bound" on operations. E.g. for tiered storage uploads[^1], for reading from local storage[^2], etc. We also do often convert from closed to opened offset intervals representations. E.g. committed offset to LSO and the other way around. When combined, these can result in unexpected behaviors. In particular, if on a read path the max offset is specified as model::offset_max() but at lower level this is converted into an exclusive offset by calling next_offset(model::offset::max()), the result is model::offset::min() aka -2^63. This is dangerous. Let's instead saturate the offset similar to how we saturate prev_offset. We also have a few cases where we just do `o + model::offset(1)`. These should be refactored to use next_offset too. This isn't fixing any existing known bug. Discovered this while trying to rewrite some logic related to tiered storage uploads. [^1]: https://github.com/redpanda-data/redpanda/blob/79bf7eed6e04da1d0987b5abd719c4b289dde761/src/v/archival/ntp_archiver_service.cc#L1656 [^2]: https://github.com/redpanda-data/redpanda/blob/79bf7eed6e04da1d0987b5abd719c4b289dde761/src/v/cluster/migrations/tx_manager_migrator.cc#L219
1 parent eb003fe commit 8c3777b

File tree

1 file changed

+2
-0
lines changed

1 file changed

+2
-0
lines changed

src/v/model/fundamental.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ inline constexpr model::offset_delta offset_delta_cast(model::offset r) {
239239
inline constexpr model::offset next_offset(model::offset o) {
240240
if (o < model::offset{0}) {
241241
return model::offset{0};
242+
} else if (o == model::offset::max()) {
243+
return model::offset::max();
242244
}
243245
return o + model::offset{1};
244246
}

0 commit comments

Comments
 (0)