Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align KIP-79; Redpanda shouldn't return negative value when retention expires #4308

Closed
daisukebe opened this issue Apr 18, 2022 · 4 comments · Fixed by #4322
Closed

Align KIP-79; Redpanda shouldn't return negative value when retention expires #4308

daisukebe opened this issue Apr 18, 2022 · 4 comments · Fixed by #4322
Assignees
Labels
kind/bug Something isn't working

Comments

@daisukebe
Copy link
Contributor

Version & Environment

Redpanda version: v21.11.12 (also exists in the dev branch, 248e103)

What went wrong?

Current Redpanda doesn't support what KIP-79 has carried, especially

The target timestamp have to be non-negative. Otherwise an IllegalArgumentException will be thrown. If no message has a timestamp that is greater than or equals to the target time, a null will be returned.

Once a record's retention gets expired, offsetsForTimes throws java.lang.IllegalArgumentException: Invalid negative timestamp since Redpanda returns a negative value.

What should have happened instead?

Redpanda should return non-negative value

How to reproduce the issue?

  1. Let a record's retention get expired
  2. Call offsetsForTimes

Additional information

java.lang.IllegalArgumentException: Invalid negative timestamp
	at org.apache.kafka.clients.consumer.OffsetAndTimestamp.<init>(OffsetAndTimestamp.java:39)
	at org.apache.kafka.clients.consumer.internals.Fetcher.offsetsForTimes(Fetcher.java:522)
	at org.apache.kafka.clients.consumer.KafkaConsumer.offsetsForTimes(KafkaConsumer.java:2116)
	at org.apache.kafka.clients.consumer.KafkaConsumer.offsetsForTimes(KafkaConsumer.java:2080)
	at redpanda.Offset.main(Offset.java:42)
@daisukebe daisukebe added the kind/bug Something isn't working label Apr 18, 2022
@daisukebe daisukebe changed the title Align KIP-79 Redpanda shouldn't return negative value when retention expires Apr 18, 2022
@daisukebe daisukebe changed the title Redpanda shouldn't return negative value when retention expires Align KIP-79; Redpanda shouldn't return negative value when retention expires Apr 18, 2022
@piyushredpanda piyushredpanda added this to the v21.11.13 milestone Apr 18, 2022
@michaelandrepearce
Copy link

michaelandrepearce commented Apr 18, 2022

tbh clear its not just for retention expired, if you set a search for a time where the partition doesnt have data after or at the timestamp it also incorrectly returns -1 and shouldn't it should return as null

@michaelandrepearce
Copy link

From KIP-79 "If no message has a timestamp that is greater than or equals to the target time, a null will be returned"

@LenaAn
Copy link
Contributor

LenaAn commented Apr 19, 2022

Here's my findings so far:

Kafka Protocol:
the time in ListOffsetRequest means different things in v0 and starting v1

In ListOffsetRequest/ListOffsetResponse v0, we return a list of offsets which is smaller than or equals to the target time.

Starting v1 (from KIP-79):

Look up the offsets for the given partitions by timestamp. The returned offset for each partition is the earliest offset whose timestamp is greater than or equals to the given timestamp in the corresponding partition. If no message has a timestamp that is greater than or equals to the target time, a null will be returned

Current behavior of Redpanda:

  1. We don't support v0 (but we claim to), we always try to return the earliest offset whose timestamp is greater than or equals to the given timestamp.
  2. If there's no such offset, we return last_stable_offset with timestamp(-1)

LenaAn pushed a commit to LenaAn/redpanda that referenced this issue Apr 19, 2022
ListOffest returns the earliest offset with timestamp greater or equal
to the timestamp specifed. If no such an offset is found, ListOffset
should return null. See KIP-79

Fixes: redpanda-data#4308
LenaAn pushed a commit to LenaAn/redpanda that referenced this issue Apr 19, 2022
ListOffest returns the earliest offset with timestamp greater or equal
to the timestamp specifed. If no such an offset is found, ListOffset
should returns an empty response: offset(-1) with timestamp(-1) and
error_code.none. See KIP-79

Fixes: redpanda-data#4308
LenaAn pushed a commit to LenaAn/redpanda that referenced this issue Apr 19, 2022
ListOffest returns the earliest offset with timestamp greater or equal
to the timestamp specifed. If no such an offset is found, ListOffset
should returns an empty response: offset(-1) with timestamp(-1) and
error_code.none. See KIP-79

Fixes: redpanda-data#4308
(cherry picked from commit 6e5ca96)
@LenaAn
Copy link
Contributor

LenaAn commented Apr 19, 2022

/backport v21.11.x

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants