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

KAFKA-18341: Remove KafkaConfig GroupType config check and warn log #18320

Merged
merged 5 commits into from
Jan 13, 2025

Conversation

m1a2st
Copy link
Contributor

@m1a2st m1a2st commented Dec 26, 2024

Jira: https://issues.apache.org/jira/browse/KAFKA-18341

As we will remove zookeeper, we don't need this warn log when we start server

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community core Kafka Broker small Small PRs labels Dec 26, 2024
Copy link

github-actions bot commented Jan 3, 2025

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

@@ -542,15 +542,7 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
if (!protocols.contains(GroupType.CLASSIC)) {
throw new ConfigException(s"Disabling the '${GroupType.CLASSIC}' protocol is not supported.")
}
if (protocols.contains(GroupType.CONSUMER)) {
if (processRoles.isEmpty || !isNewGroupCoordinatorEnabled) {
Copy link
Member

Choose a reason for hiding this comment

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

I think that we need to keep the second part of the condition.

@@ -542,15 +542,7 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
if (!protocols.contains(GroupType.CLASSIC)) {
throw new ConfigException(s"Disabling the '${GroupType.CLASSIC}' protocol is not supported.")
}
if (protocols.contains(GroupType.CONSUMER)) {
if (processRoles.isEmpty || !isNewGroupCoordinatorEnabled) {
warn(s"The new '${GroupType.CONSUMER}' rebalance protocol is only supported in KRaft cluster with the new group coordinator.")
Copy link
Member

Choose a reason for hiding this comment

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

We could remove in KRaft cluster

if (protocols.contains(GroupType.SHARE)) {
if (processRoles.isEmpty || !isNewGroupCoordinatorEnabled) {
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

@dajac dajac removed needs-attention triage PRs from the community labels Jan 9, 2025
Copy link
Member

@dajac dajac left a comment

Choose a reason for hiding this comment

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

lgtm

@dajac dajac merged commit 0b60d08 into apache:trunk Jan 13, 2025
9 checks passed
dajac pushed a commit that referenced this pull request Jan 13, 2025
…18320)

As ZK mode is being removed for 4.0, we don't need this check anymore.

Reviewers: David Jacot <djacot@confluent.io>
@dajac
Copy link
Member

dajac commented Jan 13, 2025

Merged to trunk and 4.0.

tedyyan pushed a commit to tedyyan/kafka that referenced this pull request Jan 13, 2025
…pache#18320)

As ZK mode is being removed for 4.0, we don't need this check anymore.

Reviewers: David Jacot <djacot@confluent.io>
askew pushed a commit to askew/kafka that referenced this pull request Jan 23, 2025
…pache#18320)

As ZK mode is being removed for 4.0, we don't need this check anymore.

Reviewers: David Jacot <djacot@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved core Kafka Broker small Small PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants