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

Metadata refresh should skip updates when metadata response is empty #2664

Closed
HaoSunUber opened this issue Oct 8, 2023 · 3 comments · Fixed by #2672
Closed

Metadata refresh should skip updates when metadata response is empty #2664

HaoSunUber opened this issue Oct 8, 2023 · 3 comments · Fixed by #2672
Labels

Comments

@HaoSunUber
Copy link
Contributor

HaoSunUber commented Oct 8, 2023

Description

After my PR, the Sarama client will always pick up a least loaded broker from cached brokers to fetch metadata. Then the metadata response will update our cached brokers (updateBroker).

But if the chosen broker returns a response with empty brokers, updateBroker will clear our cached brokers into empty. The client will fall back to the seed broker to refresh metadata in the next metadata refresh because cached brokers are empty. This is what we want to avoid using the seed broker after bootstrap due to the stale metadata issue (issues).

Technically, the broker should not return an empty broker in the response. But we do come across this situation where the broker returns empty brokers without any error and emptys our cached brokers. It looks like a bug in Kafka. The Java client skips the empty response to update the metadata cache (code).

In Sarama, we should also implement the same logic to skip the update when the response is empty and move to the next candidate broker.

I can make a PR if you think it is reasonable.

@dnwe
Copy link
Collaborator

dnwe commented Oct 9, 2023

@HaoSunUber thanks for reporting this. It does look like a bug that could be easy to hit during a rolling restart of a kafka cluster. I agree that we should match the Java client and ignore empty metadata like this

@dnwe dnwe added the bug label Oct 9, 2023
@HaoSunUber
Copy link
Contributor Author

Thanks. I will do a change soon

HaoSunUber added a commit to HaoSunUber/sarama that referenced this issue Oct 13, 2023
…nse is empty

Fix IBM#2664

We should skip the metadata refresh if the startup phase broker returns empty brokers in metadata response. The Java client skips the empty response to update the metadata cache (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java#L1149) and we should make a feature parity in Sarama too

Signed-off-by: Hao Sun <haos@uber.com>
dnwe pushed a commit that referenced this issue Oct 13, 2023
We should skip the metadata refresh if the startup phase broker returns empty brokers in metadata response. The Java client skips the empty response to update the metadata cache (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java#L1149) and we should make a feature parity in Sarama too

Fixes #2664

Signed-off-by: Hao Sun <haos@uber.com>
@HaoSunUber
Copy link
Contributor Author

Thanks @dnwe for your quick response. Would you mind building a new release so that we can use it in our company?

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

Successfully merging a pull request may close this issue.

2 participants