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

Support Fetch v10 protocol (minimal) #792

Merged
merged 4 commits into from
Jul 24, 2020
Merged

Conversation

ankon
Copy link
Contributor

@ankon ankon commented Jun 25, 2020

This provides the protocol definitions for Fetch v10 (KIP-110: ZStd Support), without any additional changes to actually support consuming or producing zstd compressed content.

For producing: This will require updating the produce APIs as well, see https://github.com/tulios/kafkajs/projects/2 for the bigger picture.
For consuming: This might actually work just fine, if one provides a suitable codec through the regular extension mechanism.

This is mainly here as a prerequisite for KIP-392 support (#713), which requires Fetch v11 messages.

Note: This PR is floating on top of the Fetch v9 changes (#778), and should stay a draft until we can merge that one first.

@ankon
Copy link
Contributor Author

ankon commented Jun 25, 2020

Test failures are unrelated, see #786

@ankon ankon force-pushed the pr/fetch-v10 branch 2 times, most recently from a50eaca to b9effb0 Compare June 26, 2020 14:18
This version is identical to v9, but allows for the compression type to be 'zstd', and allows
a new error code to be returned when fetching. We already had earlier code to inject a zstd
codec through the regular codec extension mechanism, this commit now simply makes it possible
to actually trigger that code.
@ankon ankon marked this pull request as ready for review July 20, 2020 14:57
@Nevon
Copy link
Collaborator

Nevon commented Jul 21, 2020

So this PR is pretty simple and uncontroversial, but I want to make sure that I understand what the implications are. By supporting Fetch V10, the only thing that will happen is that if a partition is using zstd compression, we will get back data (that we can't decompress without a codec), rather than an UNSUPPORTED_COMPRESSION_TYPE error?

@ankon
Copy link
Contributor Author

ankon commented Jul 21, 2020

Yes, this is my understanding as well. KafkaJS supports the v2 format from the KIP, so the discussion about down-conversion shouldn't apply at all. By sending a Fetch v10 request we're indicating that we understand a response that talks about zstd (which we do-ish, assuming we had a codec).

@Nevon Nevon merged commit 1f442e9 into tulios:master Jul 24, 2020
@ankon ankon deleted the pr/fetch-v10 branch July 24, 2020 15:28
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.

2 participants