kafka: fix timequery failing for empty topics #19937
Conversation
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/50492#019036c9-7df0-4dae-9f74-4a0bbe00f579 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/50519#01903b62-bc5b-49c3-bb11-5e94ec7e05d0 |
|
Failures unrelated:
|
| auto min_offset = kafka_partition->start_offset(); | ||
| auto max_offset = model::prev_offset(offset); | ||
|
|
||
| // Empty topic. |
|
new failures in https://buildkite.com/redpanda/redpanda/builds/50492#019039a7-e1c9-4c3d-bde6-ae8fd8a700b3: new failures in https://buildkite.com/redpanda/redpanda/builds/50492#019039fe-e358-42eb-9329-96e673999845: |
This bug was introduced accidentally while trying to fix another off-by-one bug in commit ref 8f2de96. I forgot to account that for an empty topic the "start offset" is equal to "last offset" so calling `model::prev_offset` would result in an offset below the start which is invalid and throws an exception if passed downstream. To avoid the problem, we short-circuit with a "offset not found" response straight away if that's the case. redpanda-data@8f2de96
nvartolomei
force-pushed
the
nv/timequery-emtpy-topic
branch
from
3e11d09 to
1a080ee
Compare
|
Force-pushed to add a new test case where the topic is empty as a result of trim-prefix command. Also rebased. |
Forgot to re-enable after fixing a bunch of timequery bugs.
| co_return list_offsets_response::make_partition( | ||
| ktp.get_partition(), | ||
| model::timestamp(-1), | ||
| model::offset(-1), | ||
| kafka_partition->leader_epoch()); |
There was a problem hiding this comment.
Would it make sense to push this check into cluster::partition too / instead?
There was a problem hiding this comment.
We need to refactor this to use the bounded interval. Need to make it support Kafka offsets too first iirc.
Will merge it like this for now to make progress on the backports and will try to clean it up in a separate PR.
thanks for pointing it out!
|
/backport v24.1.x |
|
/backport v23.3.x |
This bug was introduced accidentally while trying to fix another off-by-one bug in commit ref 8f2de96. I forgot to account that for an empty topic the "start offset" is equal to "last offset" so calling
model::prev_offsetwould result in an offset below the start which is invalid and throws an exception if passed downstream.To avoid the problem, we short-circuit with a "offset not found" response straight away if that's the case.
8f2de96
Backports Required
Release Notes
Bug Fixes