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

feat: upgrade go kafka client sarama version v1.42.2 to v1.43.3 #1772

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

WillMeng
Copy link

@WillMeng WillMeng commented Sep 20, 2024

support kafka version 3.8.0
issue #1767

@CLAassistant
Copy link

CLAassistant commented Sep 20, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ WillMeng
❌ mengfanwei


mengfanwei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@shalousun
Copy link
Collaborator

@WillMeng For this client upgrade, also change the default Version configuration in flusher_kafka_v2.go to 2.1.0. This is the minimum Message protocol version supported by subsequent new versions of Kafka; versions below 2.1.0 will be removed from support by Kafka. Update the corresponding documentation in flusher-kafka_v2.md.

Additionally, modify the default size description for MaxMessageBytes in docs/cn/plugins/flusher/flusher-kafka_v2.md. After the upgrade, the default size is 1048576, rather than the previous 1000000.

@WillMeng
Copy link
Author

@WillMeng For this client upgrade, also change the default Version configuration in flusher_kafka_v2.go to 2.1.0. This is the minimum Message protocol version supported by subsequent new versions of Kafka; versions below 2.1.0 will be removed from support by Kafka. Update the corresponding documentation in flusher-kafka_v2.md.

Additionally, modify the default size description for MaxMessageBytes in docs/cn/plugins/flusher/flusher-kafka_v2.md. After the upgrade, the default size is 1048576, rather than the previous 1000000.

Thanks reply.
I campared Sarama version v1.42.2 https://github.com/IBM/sarama/blob/v1.42.2/utils.go#L266 and v1.43.3 https://github.com/IBM/sarama/blob/v1.43.3/utils.go#L266, the default version both 2.1.0. I check flusher_kafka_v2.go default version is 1.0.0, so need I update doc?
I will update MaxMessageBytes in doc

@shalousun
Copy link
Collaborator

@WillMeng For this client upgrade, also change the default Version configuration in flusher_kafka_v2.go to 2.1.0. This is the minimum Message protocol version supported by subsequent new versions of Kafka; versions below 2.1.0 will be removed from support by Kafka. Update the corresponding documentation in flusher-kafka_v2.md.
Additionally, modify the default size description for MaxMessageBytes in docs/cn/plugins/flusher/flusher-kafka_v2.md. After the upgrade, the default size is 1048576, rather than the previous 1000000.

Thanks reply. I campared Sarama version v1.42.2 https://github.com/IBM/sarama/blob/v1.42.2/utils.go#L266 and v1.43.3 https://github.com/IBM/sarama/blob/v1.43.3/utils.go#L266, the default version both 2.1.0. I check flusher_kafka_v2.go default version is 1.0.0, so need I update doc? I will update MaxMessageBytes in doc

Yes, it needs to be updated, Kafka upgrades will not be compatible.

@WillMeng
Copy link
Author

WillMeng commented Sep 23, 2024

@WillMeng For this client upgrade, also change the default Version configuration in flusher_kafka_v2.go to 2.1.0. This is the minimum Message protocol version supported by subsequent new versions of Kafka; versions below 2.1.0 will be removed from support by Kafka. Update the corresponding documentation in flusher-kafka_v2.md.
Additionally, modify the default size description for MaxMessageBytes in docs/cn/plugins/flusher/flusher-kafka_v2.md. After the upgrade, the default size is 1048576, rather than the previous 1000000.

Thanks reply. I campared Sarama version v1.42.2 https://github.com/IBM/sarama/blob/v1.42.2/utils.go#L266 and v1.43.3 https://github.com/IBM/sarama/blob/v1.43.3/utils.go#L266, the default version both 2.1.0. I check flusher_kafka_v2.go default version is 1.0.0, so need I update doc? I will update MaxMessageBytes in doc

Yes, it needs to be updated, Kafka upgrades will not be compatible.

Sorry, I reviewed the code about Sarama 1.43.3, it currently only supports Kafka 3.6.0 and does not support Kafka 3.8.0. We used kafka version 3.8.0, ilogtail kafka config Version: 3.3.1, it works well. So I don't know how to update the doc.

The Kafka protocol version is not strongly associated with the Kafka release version. The Kafka protocol version primarily represents the message protocol version supported by Kafka. In fact, current high-version Kafka instances all support Kafka protocol version 2.1.0, and future Kafka 4.0 will also support Kafka protocol version 2.1.0. As long as the Kafka protocol version does not exceed the release version of Kafka, it should work correctly.

@shalousun
Copy link
Collaborator

shalousun commented Sep 24, 2024

@WillMeng ilogtail flusher kafka v2之前默认设置的协议版本号是1.0.0,高版本的kafka已经声明了不再支持2.1.0以下的message协议版本,如果配置写入高版本的kafka是将Version指定kafka message version为2.1.0以上的即可。因此本次升级客户端可以修改下设置的默认值为2.1.0, 这样高于2.1.0以上的kafka不配置Version默认也是可以正常用的。

@WillMeng
Copy link
Author

@WillMeng ilogtail flusher kafka v2之前默认设置的协议版本号是1.0.0,高版本的kafka已经声明了不再支持2.1.0以下的message协议版本,如果配置写入高版本的kafka是将Version指定kafka message version为2.1.0以上的即可。因此本次升级客户端可以修改下设置的默认值为2.1.0, 这样高于2.1.0以上的kafka不配置Version默认也是可以正常用的。

I have updated the doc, please review it, thanks

@Takuka0311
Copy link
Collaborator

可以麻烦您把主干先merge一下吗?让测试跑通就可以合入了

@shalousun
Copy link
Collaborator

可以先rebase下这个pr分支,然后使用再基于main分支rebase下

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.

4 participants