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 NOT_COORDINATOR warnings for Kafka consumer offset commit requests #1693

Conversation

justinweng-instaclustr
Copy link
Collaborator

@justinweng-instaclustr justinweng-instaclustr commented Jul 18, 2024

This PR fixes the NOT_COORDINATOR warning when Kafka consumers try to commit offsets. It's fixed by routing the OffsetCommit requests to the group coordinator in the KafkaSinkCluster method route_requests().

It also adds a new Kafka integration test case produce_consume_commit_offsets_partitions1 which calls commit_sync(&offsets) to commit the offset after a valid consume, and calls committed to verify the offset has been successfully committed (refs: Kafka Java driver commitSync(), Kafka Java driver committed(), Kafka C++ driver commit(), Kafka C++ driver committed_offsets()).

commit_sync was chosen over commit_async due to the following reasons:

  • We need to check if the offset has been committed after sending the offset commit request, and hence need the commit_sync to block until the commit succeeds.

  • It may be possible to wrap committed() in the OffsetCommitCallback for the commitAsync() method so that the check on committed offsets can be triggered after commitAsync() completes. But it seems complicated to implement and probably not worth the effort.

Closes #1687

Copy link

codspeed-hq bot commented Jul 18, 2024

CodSpeed Performance Report

Merging #1693 will not alter performance

Comparing justinweng-instaclustr:1687-kafka-consumers-seeing-not_coordinator-warnings (9fef022) with main (6ec2022)

Summary

✅ 39 untouched benchmarks

@justinweng-instaclustr justinweng-instaclustr changed the title Add kafka int test to reproduce NOT_COORDINATOR warning Fix NOT_COORDINATOR warnings for Kafka consumer offset commit requests Jul 22, 2024
@justinweng-instaclustr justinweng-instaclustr marked this pull request as ready for review July 23, 2024 01:19
@rukai rukai mentioned this pull request Jul 23, 2024
Copy link
Member

@rukai rukai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work!
The extra test coverage will be really handy.

@justinweng-instaclustr justinweng-instaclustr merged commit dfdd714 into shotover:main Jul 23, 2024
41 checks passed
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.

Kafka consumers through shotover frequently seeing NOT_COORDINATOR warnings
3 participants