KAFKA-18360: Remove zookeeper configurations #18566

cmccabe · 2025-01-15T21:46:14Z

Remove the zookeeper configurations in ZkConfig.java.

Remove broker.id.generation.enable and reserved.broker.max.id, which are not used in KRaft mode.

Remove inter.broker.protocol.version, which is not used in KRaft mode.

core/src/main/scala/kafka/server/KafkaConfig.scala

ijuma · 2025-01-17T13:27:00Z

Can you fix the conflicts?

core/src/main/scala/kafka/server/KafkaConfig.scala

tools/src/test/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommandTest.java

core/src/test/scala/unit/kafka/server/KafkaApisTest.scala

chia7712 · 2025-01-20T19:19:47Z

core/src/main/scala/kafka/server/KafkaConfig.scala

@@ -221,8 +213,6 @@ class KafkaConfig private(doLog: Boolean, val props: util.Map[_, _])
  def quotaConfig: QuotaConfig = _quotaConfig

  /** ********* General Configuration ***********/
-  val brokerIdGenerationEnable: Boolean = getBoolean(ServerConfigs.BROKER_ID_GENERATION_ENABLE_CONFIG)
-  val maxReservedBrokerId: Int = getInt(ServerConfigs.RESERVED_BROKER_MAX_ID_CONFIG)
  var brokerId: Int = getInt(ServerConfigs.BROKER_ID_CONFIG)


Pull request #18478 removes the broker.id configuration, but this pull request does not. Should we continue treating broker.id as synonymous with node.id in Kafka version 4.x? The documentation states that node.id is mandatory when running in KRaft mode. Therefore, it's likely safe to remove broker.id from version 4.x, as Kafka servers in KRaft mode should already be configured with node.id.

If the behavior in 3.x is that the broker fails to start in KRaft mode unless node.id is set, then it would be fine to remove broker.id. But we should double check to make sure we didn't make an incompatible change in 4.0/trunk.

If the behavior in 3.x is that the broker fails to start in KRaft mode unless node.id is set

There's a potential regression if we remove broker.id because users can currently set it instead of node.id. However, this conflicts with our documentation, which states that node.id is mandatory. To address this, we could deprecate broker.id in version 4.x and then remove it entirely in version 5.x.

ping @dajac if the approach of deprecating broker.id is acceptable.

Deprecating broker.id has to be done via a KIP. Since there is no rush for that one, let's focus on making sure we don't break the 3.x behavior for now.

@FrankYang0529 Let's retain broker.id in issue #18478. Additionally, we should update the documentation to clarify that node.id and broker.id are synonymous. This will help prevent user confusion regarding their relationship.

Thanks for the confirmation. I will update my PR.

@ijuma @chia7712 do you think that we should add a warning message when users use broker.id? To let them know the configuration will be removed in 5.0.

do you think that we should add a warning message when users use broker.id? To let them know the configuration will be removed in 5.0.

broker.id is not deprecated in 4.0 (as @ijuma mentioned it needs a KIP), so the warning message is unnecessary to me. I open https://issues.apache.org/jira/browse/KAFKA-18628 to discuss the broker.id

Yeah, we should discuss the node.id / broker.id thing in a separate JIRA or PR, I think.

chia7712 · 2025-01-29T19:02:49Z

@cmccabe could you please fix the conflicts? I hope this is the last PR required to remove zk configuration

clolov · 2025-02-05T10:04:10Z

Heya @cmccabe, if you are short on cycles I can help getting this over the line?

cmccabe · 2025-02-05T17:32:50Z

@ijuma @clolov @chia7712 the conflicts should be fixed now. sorry for the delay!

chia7712

LGTM and I open some follow-ups

chia7712 · 2025-02-05T17:38:31Z

core/src/main/scala/kafka/server/KafkaRaftServer.scala

@@ -182,8 +182,7 @@ object KafkaRaftServer {
    }

    // Load the BootstrapMetadata.
-    val bootstrapDirectory = new BootstrapDirectory(config.metadataLogDir,
-      Optional.ofNullable(config.interBrokerProtocolVersionString))
+    val bootstrapDirectory = new BootstrapDirectory(config.metadataLogDir, Optional.empty())


open https://issues.apache.org/jira/browse/KAFKA-18740 to cleanup BootstrapDirectory as follow-up

core/src/main/scala/kafka/server/KafkaConfig.scala

chia7712 · 2025-02-05T17:46:53Z

server/src/main/java/org/apache/kafka/server/config/ReplicationConfigs.java

@@ -127,13 +125,6 @@ public class ReplicationConfigs {
            "thread to trigger election periodically (default is 5 minutes). Please run `kafka-leader-election.sh` with `unclean` option " +
            "to trigger the unclean leader election immediately if needed.</p>";

-    public static final String INTER_BROKER_PROTOCOL_VERSION_CONFIG = "inter.broker.protocol.version";


open https://issues.apache.org/jira/browse/KAFKA-18741 to document it

chia7712

the failed test testFormatWithReleaseVersionDefault is using inter.broker.protocol.version so it should be removed by this PR.

testFormatWithReleaseVersionDefaultAndReleaseVersion also sets the inter.broker.protocol.version. We can remove the line as it does not impact the test

chia7712 · 2025-02-06T05:55:37Z

core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala

@@ -187,7 +186,7 @@ class KafkaConfigTest {
    val advertisedHostName = "routable-host"
    val advertisedPort = 1234

-    val props = TestUtils.createBrokerConfig(0)
+    val props = createDefaultConfig()


this createDefaultConfig does not return a valid config (https://github.com/apache/kafka/pull/18547/files#r1944140562)

@mingyen066 will fix it by a minor patch

chia7712 · 2025-02-06T09:19:45Z

@cmccabe I push a commit to include following changes.

remove inter.broker.protocol.version from testFormatWithReleaseVersionDefaultAndReleaseVersion
remove testFormatWithReleaseVersionDefault

I apologize for making minor changes to your PR. These changes are necessary to unblock subsequent work for the 4.0 release. I will merge this PR once it passes QA.

Remove broker.id.generation.enable and reserved.broker.max.id, which are not used in KRaft mode. Remove inter.broker.protocol.version, which is not used in KRaft mode. Reviewers: PoAn Yang <[email protected]>, Ismael Juma <[email protected]>, Chia-Ping Tsai <[email protected]>

github-actions bot added triage PRs from the community core Kafka Broker labels Jan 15, 2025

ijuma reviewed Jan 16, 2025

View reviewed changes

core/src/main/scala/kafka/server/KafkaConfig.scala Show resolved Hide resolved

KAFKA-18360: Remove zookeeper configurations

cc4d592

cmccabe force-pushed the KAFKA-18360 branch from 58e90eb to cc4d592 Compare January 16, 2025 23:41

github-actions bot added tools and removed triage PRs from the community labels Jan 16, 2025

ijuma mentioned this pull request Jan 17, 2025

KAFKA-18366 Remove KafkaConfig.interBrokerProtocolVersion #18597

Closed

3 tasks

ijuma reviewed Jan 17, 2025

View reviewed changes

core/src/main/scala/kafka/server/KafkaConfig.scala Outdated Show resolved Hide resolved

ijuma reviewed Jan 17, 2025

View reviewed changes

core/src/main/scala/kafka/server/KafkaConfig.scala Outdated Show resolved Hide resolved

ijuma reviewed Jan 17, 2025

View reviewed changes

tools/src/test/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommandTest.java Show resolved Hide resolved

ijuma reviewed Jan 17, 2025

View reviewed changes

core/src/test/scala/unit/kafka/server/KafkaApisTest.scala Show resolved Hide resolved

chia7712 reviewed Jan 20, 2025

View reviewed changes

cmccabe added 2 commits January 24, 2025 16:57

Merge branch 'trunk' into KAFKA-18360

5d02cdf

updates

fd7d123

Merge branch 'trunk' into KAFKA-18360

26fc14b

Clean up comments and names a bit

91c1df3

chia7712 approved these changes Feb 5, 2025

View reviewed changes

chia7712 reviewed Feb 6, 2025

View reviewed changes

chia7712 added 2 commits February 6, 2025 17:15

Merge branch 'trunk' into KAFKA-18360

162ce97

remove testFormatWithReleaseVersionDefault

dca3009

chia7712 merged commit b2b2408 into apache:trunk Feb 6, 2025
9 checks passed

chia7712 mentioned this pull request Feb 10, 2025

KAFKA-18601: Assume a baseline of 3.3 for server protocol versions #18845

Merged

3 tasks

Yunyung mentioned this pull request Feb 14, 2025

KAFKA-18740: Remove BootstrapDirectory#ibp #18905

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-18360: Remove zookeeper configurations #18566

KAFKA-18360: Remove zookeeper configurations #18566

cmccabe commented Jan 15, 2025

ijuma commented Jan 17, 2025

chia7712 Jan 20, 2025

ijuma Jan 20, 2025

chia7712 Jan 21, 2025

chia7712 Jan 21, 2025

ijuma Jan 22, 2025 •

edited

Loading

chia7712 Jan 23, 2025

FrankYang0529 Jan 23, 2025

FrankYang0529 Jan 23, 2025

chia7712 Jan 23, 2025

cmccabe Jan 25, 2025

chia7712 commented Jan 29, 2025

clolov commented Feb 5, 2025

cmccabe commented Feb 5, 2025

chia7712 left a comment

chia7712 Feb 5, 2025

chia7712 Feb 5, 2025

chia7712 left a comment

chia7712 Feb 6, 2025

chia7712 commented Feb 6, 2025

KAFKA-18360: Remove zookeeper configurations #18566

KAFKA-18360: Remove zookeeper configurations #18566

Conversation

cmccabe commented Jan 15, 2025

ijuma commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijuma Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 commented Jan 29, 2025

clolov commented Feb 5, 2025

cmccabe commented Feb 5, 2025

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 commented Feb 6, 2025

ijuma Jan 22, 2025 •

edited

Loading