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

Fix invalid modifications to FindCoordinator responses #1077

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

rukai
Copy link
Member

@rukai rukai commented Mar 13, 2023

The problem was that the encoder expects protocols >= 4 to have all the removed fields set to 0 (or equivalent default value)
We were just setting all the port fields regardless of version causing the encoder to return an error when a modern protocol was used.
Now we only set the port fields that are actually available in the specific version of the message.

You can verify this fixes the issue by simultaneously running these scripts that come with kafka against test-configs/kafka/passthrough/topology.yaml:

kafka-console-producer.sh  --topic foo --bootstrap-server 127.0.0.1:9192

and

kafka-console-consumer.sh --topic foo --bootstrap-server 127.0.0.1:9192

Then sending a message through the producer and observing it appear in the consumer.
Before this fix, we would hit the encode error and nothing would appear in the consumer.

@rukai rukai enabled auto-merge (squash) March 13, 2023 23:07
@rukai rukai merged commit 7df6e52 into shotover:main Mar 14, 2023
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.

3 participants