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

Differentiate producer/consumer configuration #16

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

rodaine
Copy link
Member

@rodaine rodaine commented Oct 7, 2024

The producer/consumer code was sharing the same configuration, which means the producer was being initialized as a consumer as well. If the producer starts first, it joins the same group and is assigned the only partition for the topic. When the consumer eventually comes online, it joins the group but is not assigned the partition, appearing to be stuck. This patch bifurcates the configuration on whether or not the Kafka client being stood up should be a consumer. Also, the PR adds a few hygienic options that should always be set for consumers (read committed, require stable offsets).

@rodaine rodaine requested review from akshayjshah and bufdev October 7, 2024 16:12
@@ -28,14 +28,21 @@ type Config struct {
}

// NewKafkaClient returns a new franz-go Kafka Client for the given Config.
func NewKafkaClient(config Config) (*kgo.Client, error) {
func NewKafkaClient(config Config, consumer bool) (*kgo.Client, error) {
Copy link
Member

Choose a reason for hiding this comment

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

To unblock, approving PR, but would prefer we split this out into NewKafkaConsumerClient and NewKafkaProducerClient for demo clarity - I can do this in a follow-up though

@rodaine rodaine merged commit 40a12bb into main Oct 7, 2024
3 checks passed
@rodaine rodaine deleted the rodaine/differentiate-consumer-producer-config branch October 7, 2024 16:18
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