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

Check for insufficient data when we try to get the records. #998

Merged
merged 1 commit into from
Dec 5, 2017
Merged

Check for insufficient data when we try to get the records. #998

merged 1 commit into from
Dec 5, 2017

Conversation

wladh
Copy link
Contributor

@wladh wladh commented Dec 5, 2017

In some corner cases (like v2 records produced on the topic but the
sarama consumer uses v0 Fetch API with a limit less than one record size)
we might receive a truncated record batch. We want to be able to make
progress by increasing the fetch limit, so don't return an error, but
rater mark the batch as partial trailing record.
Fixes issue #962

In some corner cases (like v2 records produced on the topic but the
sarama consumer uses v0 Fetch API with a limit less than one record size)
we might receive a truncated record batch. We want to be able to make
progress by increasing the fetch limit, so don't return an error, but
rater mark the batch as partial trailing record.
@tcrayford
Copy link
Contributor

@wladh what does the java client do here?

@eapache
Copy link
Contributor

eapache commented Dec 5, 2017

This fix makes sense to me, though I'm also curious what the java client does here.

@eapache
Copy link
Contributor

eapache commented Dec 5, 2017

Thanks for digging into this!

@eapache eapache merged commit 8f05b2f into IBM:master Dec 5, 2017
@wladh
Copy link
Contributor Author

wladh commented Dec 5, 2017

From what I gather (not familiar with Kafka codebase) the java client doesn't have this incremental fetch size logic. So it basically fails the way sarama was failing before this commit: https://github.com/apache/kafka/blob/4b8a29f12a142d02be0b64eac71975b6a129d04a/clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java#L840-L855

@tcrayford
Copy link
Contributor

@wladh that doesn't line up with my testing, which seemed to indicate everything "just worked" with the jvm clients.

@wladh
Copy link
Contributor Author

wladh commented Dec 5, 2017

@tcrayford like I said I'm not familiar with the code base and have never used the Java client (this was just looking at the code at HEAD in their repo, maybe things were changed) so I might be missing something. One difference that I see is that the default fetch size for Java client is 1MB, whereas sarama starts at 32KB. But I don't know how big your messages were or if you changed any of these params.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants