aiven / kafka

Mirror of Apache Kafka
Apache License 2.0
2 stars 1 forks source link

ListOffsets returns earliest offset locally, not taking remote into account #14

Open ivanyu opened 1 year ago

ivanyu commented 1 year ago

Assuming the beginning of the log is on remote

kcat -b localhost:9092 -C -t topic1 -o beginning -e -f "%o\n" | head

fetches only the locally available records, but

kcat -b localhost:9092 -C -t topic1 -o 0 -e -f "%o\n" | head

fetches records from the remote storage.

This Python

tp = TopicPartition("topic1", 0)
kc.assign([tp])
kc.seek_to_beginning(tp)
r = kc.poll(timeout_ms=1)

fetches from the remote.

The issue is that when we request the earliest offset from the broker (ListOffsets request type / beginning_offsets in Python)), it returns the earliest available locally, i.e. not counting remote.

mdedetrich commented 1 year ago

So I have created an implementation locally, wasn't too hard but I need to verify whether the returning offsets need to be ordered in some way.

mdedetrich commented 1 year ago

Here is the patch so far, going to confirm how ordering is meant to work.

Make_ListOffsets_work_with_Tiered_Storage.patch

HenryCaiHaiying commented 1 year ago

Are we moving forward with the patching? It seems it might be important for some consumers.

ivanyu commented 1 year ago

@mdedetrich did the patch work for you? It doesn't for me, the kcat test.

At least log.logStartOffset as a parameter to findOffsetByTimestamp will not produce anything, because log.logStartOffset is always (currently) higher than anything in the remote storage.

Also, based on how oldStyleOffsets is used later, it should always be 0 or 1 element, so I guess ++ won't work.

mdedetrich commented 1 year ago

Indeed this is correct, I was trying to write a test to validate this and I noticed the same. I was attempting to upstream this first and the core implementation of ListOffsets has changed compared to whats in 3.3.