Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

set sarama client KafkaVersion via config #1103

Merged
merged 2 commits into from
Oct 23, 2018
Merged

Conversation

woodsaj
Copy link
Member

@woodsaj woodsaj commented Oct 23, 2018

see issue: #1053

from https://github.com/Shopify/sarama/blob/v1.19.0/config.go#L324-L330

The version of Kafka that Sarama will assume it is running against.
Defaults to the oldest supported stable version. Since Kafka provides
backwards-compatibility, setting it to a version older than you have
will not break anything, although it may prevent you from using the
latest features. Setting it to a version greater than you are actually
running may lead to random breakage.

see issue: #1053

from  https://github.com/Shopify/sarama/blob/v1.19.0/config.go#L324-L330

```
The version of Kafka that Sarama will assume it is running against.
Defaults to the oldest supported stable version. Since Kafka provides
backwards-compatibility, setting it to a version older than you have
will not break anything, although it may prevent you from using the
latest features. Setting it to a version greater than you are actually
running may lead to random breakage.
```
Copy link
Contributor

@Dieterbe Dieterbe left a comment

Choose a reason for hiding this comment

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

  1. seems like sarama supports any delimiter between the digits, so rather than underscores, let's use dots so that the strings are actual version strings.
  2. can we link or mention the supported ones? e.g. I notice there's a kafka release 1.1.1 for which there is no protocol version in sarama (even looking at latest master, which does support v2). this could be too much work to be worth it though.

@woodsaj
Copy link
Member Author

woodsaj commented Oct 23, 2018

seems like sarama supports any delimiter between the digits

this does appear to be true, but good catch. i just assumed that passing the supportVersion names would work, but ParseKafkaVersion actually wants the semver string. eg. 0.10.0.0

for version <1.0 it is using the regex ^0\.\d+\.\d+\.\d+$ and for >= 1.0 it is using ^\d+\.\d+\.\d+$

@woodsaj
Copy link
Member Author

woodsaj commented Oct 23, 2018

I notice there's a kafka release 1.1.1 for which there is no protocol version in sarama

Sarama only uses the version number to determine what Kafka features it can use. It does this with calls like

if bc.consumer.conf.Version.IsAtLeast(V0_10_0_0) {
		request.Version = 2
	}
	if bc.consumer.conf.Version.IsAtLeast(V0_10_1_0) {
		request.Version = 3
		request.MaxBytes = MaxResponseSize
	}
	if bc.consumer.conf.Version.IsAtLeast(V0_11_0_0) {
		request.Version = 4
		request.Isolation = ReadUncommitted // We don't support yet transactions.
	}

So the version provided doesn need to be in the list of supported versions.

@woodsaj
Copy link
Member Author

woodsaj commented Oct 23, 2018

@Dieterbe this should be good now

@Dieterbe Dieterbe merged commit 5cd8b55 into master Oct 23, 2018
@Dieterbe Dieterbe deleted the saramaKafkaVersion branch October 29, 2018 09:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants