KAFKA-19719: --no-initial-controllers should not assume kraft.version=1 #20551

kevin-wu24 · 2025-09-17T17:28:38Z

Just because a controller node sets --no-initial-controllers flag does
not mean it is necessarily running kraft.version=1. The more precise
meaning is that the controller node being formatted does not know what
kraft version the cluster should be in, and therefore it is only safe to
assume kraft.version=0. Only by setting
--standalone,--initial-controllers, or --no-initial-controllers
AND not specifying the controller.quorum.voters static config, is it
known kraft.version > 0.

For example, it is a valid configuration (although confusing) to run a
static quorum definedby controller.quorum.voters but have all the
controllers format with --no-initial-controllers. In this case,
specifying --no-initial-controllers alongside a metadata version that
does not support kraft.version=1 causes formatting to fail, which is
a regression.

Additionally, the formatter should not check the kraft.version against
the release version, since kraft.version does not actually depend on any
release version. It should only check the kraft.version against the
static voters config/format arguments.

This PR also cleans up the integration test framework to match the
semantics of formatting an actual cluster.

Reviewers: TengYao Chi kitingiao@gmail.com, Kuan-Po Tseng
brandboat@gmail.com, Chia-Ping Tsai chia7712@gmail.com

…ing setting kraft version

brandboat

Should we consider deprecating --no-initial-controllers? It seems redundant and has no effect for either dynamic or static voter configurations.

frankvicky

+1 to @brandboat
After this patch, it seems that the --no-initial-controller flag doesn't provide too much value.

chia7712 · 2025-09-18T10:24:36Z

@kevin-wu24 thanks for this patch.

it is a valid configuration to run a static quorum defined
by controller.quorum.voters but have all the controllers format with
--no-initial-controllers

Excuse me, what is the purpose of having both controller.quorum.voters and --no-initial-controllers when starting a static quorum cluster?

I assumed --no-initial-controllers is used only in building a dynamic quorum cluster.

kevin-wu24 · 2025-09-18T12:55:44Z

Thanks for the reviews @chia7712 @brandboat @frankvicky. Hopefully I can clear up the intended semantics of --no-initial-controllers a bit.

Should we consider deprecating --no-initial-controllers? It seems redundant and has no effect for either dynamic or static voter configurations.

No, we should not deprecate --no-initial-controllers, since it does have an effect for a dynamic voter configuration, but not for static voter configuration. Specifically, when the static config controller.quorum.voters is not set in kafka.properties, one of --standalone, --initial-controllers, or --no-initial-controllers, must be set. Otherwise, the storage tool fails. This shows clear intention from the user to format a dynamic quorum.

Actually, when we specify --no-initial-controllers, and also do not specify controller.quorum.voters, it is safe to assume kraft.version > 0, so I will update the PR accordingly.

Excuse me, what is the purpose of having both controller.quorum.voters and --no-initial-controllers when starting a static quorum cluster?

I assumed --no-initial-controllers is used only in building a dynamic quorum cluster.

--no-initial-controllers is only used in building a dynamic quorum cluster, meaning that when a node specifies controller.quorum.voters the flag acts as a no-op. However, specifying the flag should never break formatting because the meaning of --no-initial-controllers is that "this node may not know the intended KRaft version of cluster, and will rely on KRaft to figure out the version". This flag breaking previously valid static configurations is a regression from #17836.

…me, cleanup of kafka cluster test kit

kevin-wu24 · 2025-09-18T16:20:00Z

...mmon/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java

-            StringBuilder quorumVoterStringBuilder = new StringBuilder();
-            String prefix = "";
-            for (int nodeId : nodes.controllerNodes().keySet()) {
-                quorumVoterStringBuilder.append(prefix).
-                    append(nodeId).
-                    append("@").
-                    append("localhost").
-                    append(":").
-                    append(socketFactoryManager.getOrCreatePortForListener(nodeId, controllerListenerName));
-                prefix = ",";
+            if (!standalone && initialVoterSet.isEmpty()) {
+                StringBuilder quorumVoterStringBuilder = new StringBuilder();
+                String prefix = "";
+                for (int nodeId : nodes.controllerNodes().keySet()) {
+                    quorumVoterStringBuilder.append(prefix).
+                        append(nodeId).
+                        append("@").
+                        append("localhost").
+                        append(":").
+                        append(socketFactoryManager.getOrCreatePortForListener(nodeId, controllerListenerName));
+                    prefix = ",";
+                }
+                props.put(QuorumConfig.QUORUM_VOTERS_CONFIG, quorumVoterStringBuilder.toString());
+            } else {
+                StringBuilder bootstrapServersStringBuilder = new StringBuilder();
+                String prefix = "";
+                for (int nodeId : nodes.controllerNodes().keySet()) {
+                    bootstrapServersStringBuilder.append(prefix).
+                        append("localhost").
+                        append(":").
+                        append(socketFactoryManager.getOrCreatePortForListener(nodeId, controllerListenerName));
+                    prefix = ",";
+                }
+                props.put(QuorumConfig.QUORUM_BOOTSTRAP_SERVERS_CONFIG, bootstrapServersStringBuilder.toString());


Update the integration test framework to better mirror a real cluster for dynamic quorum (i.e. voters config should not be specified and we should use bootstrap servers)

Thanks for fixing this.

chia7712

@kevin-wu24 thanks for this patch. Should we backport this fix to 4.0 and 4.1?

chia7712 · 2025-09-19T06:33:01Z

core/src/main/scala/kafka/tools/StorageTool.scala

-      (Option(initialControllers).isDefined || isStandalone)) {
+    val staticVotersEmpty = config.quorumConfig.voters().isEmpty
+    formatter.setHasDynamicQuorum(staticVotersEmpty)
+    if (!staticVotersEmpty && (Option(initialControllers).isDefined || isStandalone)) {


I still feel it is weird that users configure controller.quorum.voters with --no-initial-controllers, but you are right that we should keep the compatibility.

Should we add another warning to remind users that --no-initial-controllers should not be used with static voters even though it is no-op?

Should we add another warning to remind users that --no-initial-controllers should not be used with static voters even though it is no-op?

I can add it to the flag's help output.

chia7712 · 2025-09-19T08:13:21Z

core/src/test/java/kafka/server/ReconfigurableQuorumIntegrationTest.java

            new TestKitNodes.Builder().
                setNumBrokerNodes(1).
                setNumControllerNodes(1).
-                setFeature(KRaftVersion.FEATURE_NAME, KRaftVersion.KRAFT_VERSION_1.featureLevel()).


the method setFeature is useless now. Could you please remove it?

chia7712 · 2025-09-19T08:24:06Z

...mmon/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java

+                    );
                    formatter.setInitialControllers(DynamicVoters.parse(dynamicVotersBuilder.toString()));
                }
+                formatter.setHasDynamicQuorum(true);


Could you please add a clarifying comment to explain the actual case here? For example, the nodeId != CONTROLLER_ID_OFFSET means the controller nodes are using --no-initial-controllers.

I don't under the added comment. This execute when standalone. Irrespective of the node id. Doesn't that mean --standalone and --no-initial-controllers?

Yeah, that's what the comment is saying. One node formats with --standalone and the others are formatting with --no-initial-controllers. For both arguments we need to call formatter.setHasDynamicQuorum(true); for this configuration.

kevin-wu24 · 2025-09-19T14:43:11Z

Thanks for the review @chia7712.

Should we backport this fix to 4.0 and 4.1?

Yeah, we should backport it since it affects both versions.

chia7712 · 2025-09-21T03:53:07Z

I send a message to 4.0.1 RC thread (https://lists.apache.org/thread/4gsgcdgybyrnwg3hodnlyxg33ckj30wy). wait for the response from @clolov

FrankYang0529

Thanks for the fix. Leave some minor comments.

FrankYang0529 · 2025-09-21T03:58:48Z

metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java

-                        "--no-initial-controllers."
+                        "Cannot set kraft.version to " + configuredKRaftVersionLevel.get() +
+                        " unless controller.quorum.voters is empty and one of the flags --standalone, " +
+                        "--initial-controllers, or -no-initial-controllers is used. " +


nit: -no-initial-controllers -> --no-initial-controllers.

Sounds good, will fix

FrankYang0529 · 2025-09-21T03:59:03Z

metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java

-                "--no-initial-controllers is used. For dynamic controllers support, try using one of " +
-                "--standalone, --initial-controllers, or --no-initial-controllers.",
+                "Cannot set kraft.version to 1 unless controller.quorum.voters is empty and " +
+                "one of the flags --standalone, --initial-controllers, or -no-initial-controllers is used. " +


FrankYang0529 · 2025-09-21T03:59:11Z

metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java

-                "--no-initial-controllers is used. For dynamic controllers support, try using one of " +
-                "--standalone, --initial-controllers, or --no-initial-controllers.",
+                "Cannot set kraft.version to 1 unless controller.quorum.voters is empty and " +
+                "one of the flags --standalone, --initial-controllers, or -no-initial-controllers is used. " +


jsancio

Thanks for the changes @kevin-wu24

jsancio · 2025-09-22T15:09:56Z

core/src/main/scala/kafka/tools/StorageTool.scala

-        if (config.quorumConfig.voters().isEmpty && formatter.initialVoters().isEmpty) {
+    if (!namespace.getBoolean("no_initial_controllers") &&
+      config.processRoles.contains(ProcessRole.ControllerRole) &&
+      staticVotersEmpty && formatter.initialVoters().isEmpty) {


I would add a newline after && and before formatter.initialVoters().isEmpt.

jsancio · 2025-09-22T15:11:12Z

core/src/main/scala/kafka/tools/StorageTool.scala


    reconfigurableQuorumOptions.addArgument("--no-initial-controllers", "-N")
-      .help("Used to initialize a server without a dynamic quorum topology.")
+      .help("Used to initialize a server without a dynamic quorum topology. When setting this flag, " +


Let's remove the word topology. It matches the phrase used for --standalone.

jsancio · 2025-09-22T15:11:49Z

core/src/main/scala/kafka/tools/StorageTool.scala

      .action(storeTrue())

    reconfigurableQuorumOptions.addArgument("--initial-controllers", "-I")
      .help("Used to initialize a server with a specific dynamic quorum topology. The argument " +


Same here. Do you mind removing the word topology?

jsancio · 2025-09-22T15:15:57Z

metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java

+    public Formatter setHasDynamicQuorum(boolean staticVotersEmpty) {
+        this.hasDynamicQuorum = staticVotersEmpty;


The parameter name should be something like hasDynamicQuorum.

jsancio · 2025-09-22T15:19:06Z

metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java

-                        "--no-initial-controllers."
+                        "Cannot set kraft.version to " + configuredKRaftVersionLevel.get() +
+                        " unless controller.quorum.voters is empty and one of the flags --standalone, " +
+                        "--initial-controllers, or --no-initial-controllers  is used. " +


Extra space after --no-initial-controllers

jsancio · 2025-09-22T15:21:44Z

metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java

+            FormatterContext formatter1 = testEnv.newFormatter();
+            formatter1.formatter.setReleaseVersion(MetadataVersion.IBP_3_8_IV0);
+            // This MV does not support kraft.version = 1
+            formatter1.formatter.setHasDynamicQuorum(emptyStaticVoters);


This is a bit odd. Why not name the test parameter hasDynamicQuorum?

jsancio · 2025-09-22T15:25:02Z

metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java

+            if (emptyStaticVoters) {
+                assertEquals((short) 1, formatter1.formatter.featureLevels.getOrDefault("kraft.version", (short) 0));
+            } else {
+                assertEquals((short) 0, formatter1.formatter.featureLevels.getOrDefault("kraft.version", (short) 1));
+            }


Don't you want to differentiate between missing value and the default in the test? Why not use featureLevels.get("kraft.version")? This comment applies to a few places in this file.

jsancio · 2025-09-22T15:27:22Z

...mmon/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java

-            StringBuilder quorumVoterStringBuilder = new StringBuilder();
-            String prefix = "";
-            for (int nodeId : nodes.controllerNodes().keySet()) {
-                quorumVoterStringBuilder.append(prefix).
-                    append(nodeId).
-                    append("@").
-                    append("localhost").
-                    append(":").
-                    append(socketFactoryManager.getOrCreatePortForListener(nodeId, controllerListenerName));
-                prefix = ",";
+            if (!standalone && initialVoterSet.isEmpty()) {
+                StringBuilder quorumVoterStringBuilder = new StringBuilder();
+                String prefix = "";
+                for (int nodeId : nodes.controllerNodes().keySet()) {
+                    quorumVoterStringBuilder.append(prefix).
+                        append(nodeId).
+                        append("@").
+                        append("localhost").
+                        append(":").
+                        append(socketFactoryManager.getOrCreatePortForListener(nodeId, controllerListenerName));
+                    prefix = ",";
+                }
+                props.put(QuorumConfig.QUORUM_VOTERS_CONFIG, quorumVoterStringBuilder.toString());
+            } else {
+                StringBuilder bootstrapServersStringBuilder = new StringBuilder();
+                String prefix = "";
+                for (int nodeId : nodes.controllerNodes().keySet()) {
+                    bootstrapServersStringBuilder.append(prefix).
+                        append("localhost").
+                        append(":").
+                        append(socketFactoryManager.getOrCreatePortForListener(nodeId, controllerListenerName));
+                    prefix = ",";
+                }
+                props.put(QuorumConfig.QUORUM_BOOTSTRAP_SERVERS_CONFIG, bootstrapServersStringBuilder.toString());


Thanks for fixing this.

jsancio · 2025-09-22T15:29:59Z

...mmon/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java

+                    );
                    formatter.setInitialControllers(DynamicVoters.parse(dynamicVotersBuilder.toString()));
                }
+                formatter.setHasDynamicQuorum(true);


I don't under the added comment. This execute when standalone. Irrespective of the node id. Doesn't that mean --standalone and --no-initial-controllers?

jsancio · 2025-09-22T15:30:45Z

test-common/test-common-runtime/src/main/java/org/apache/kafka/common/test/TestKitNodes.java

-        public Builder setFeature(String featureName, short level) {
-            this.bootstrapMetadata = bootstrapMetadata.copyWithFeatureRecord(featureName, level);
-            return this;
-        }


Thanks for finally removing this configuration.

jsancio · 2025-09-23T17:07:55Z

core/src/main/scala/kafka/tools/StorageTool.scala

-      .help("Used to initialize a server without a dynamic quorum topology. When setting this flag, " +
+      .help("Used to initialize a controller without specifying a dynamic quorum. When setting this flag, " +
        "the controller.quorum.voters config should not be set, and controller.quorum.bootstrap.servers is set instead.")
      .action(storeTrue())

    reconfigurableQuorumOptions.addArgument("--initial-controllers", "-I")
-      .help("Used to initialize a server with a specific dynamic quorum topology. The argument " +
+      .help("Used to initialize a controller with the specified dynamic quorum. The argument " +


--standalone can only be used on the controller node but --no-initial-controllers and --initial-controllers can be used in the controller and broker nodes.

I.e. you can change the message back to using the word server instead of the word controller.

chia7712

@kevin-wu24 thanks for this patch. just a couple of small comments left

chia7712 · 2025-09-24T08:30:07Z

core/src/main/scala/kafka/tools/StorageTool.scala

    Option(initialControllers).
      foreach(v => formatter.setInitialControllers(DynamicVoters.parse(v)))
    if (isStandalone) {
      formatter.setInitialControllers(createStandaloneDynamicVoters(config))


It seems that --initial-controllers becomes a no-op when --standalone is defined. Should we add a warning for this case?

If you look at the dynamic quorum arguments, you cannot specify multiple at the same time:

val reconfigurableQuorumOptions = formatParser.addMutuallyExclusiveGroup()

chia7712 · 2025-09-24T09:20:59Z

metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java

-                        configuredKRaftVersionLevel.get() +
-                        " if one of the flags --standalone, --initial-controllers, or --no-initial-controllers is used. " +
-                        "For dynamic controllers support, try removing the --feature flag for kraft.version."
+                        "Cannot set kraft.version to " + configuredKRaftVersionLevel.get() +


Would you mind using 0 instead of configuredKRaftVersionLevel.get()? for example:

throw new FormatterException( "Cannot set kraft.version to 0 if controller.quorum.voters is empty and one of the flags --standalone, " + "--initial-controllers, or --no-initial-controllers is used. For dynamic controllers support, " + "try removing the --feature flag for kraft.version." );

chia7712 · 2025-09-24T09:21:57Z

metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java

    private static final int DEFAULT_NODE_ID = 1;

    private static final Uuid DEFAULT_CLUSTER_ID = Uuid.fromString("b3dGE68sQQKzfk80C_aLZw");
+    private static final String KRAFT_VERSION = "kraft.version";


Could it be replaced by KRaftVersion.FEATURE_NAME?

chia7712 · 2025-09-24T09:42:12Z

...mmon/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java

+            StringBuilder dynamicVotersBuilder = new StringBuilder();
+            String prefix = "";
+            if (standalone) {
+                if (nodeId == TestKitDefaults.CONTROLLER_ID_OFFSET) {


The true port of the standalone controller is TestKitDefaults.BROKER_ID_OFFSET + TestKitDefaults.CONTROLLER_ID_OFFSET. We will likely not change the value of BROKER_ID_OFFSET in the future, but should we still use BROKER_ID_OFFSET + CONTROLLER_ID_OFFSET instead?

https://github.com/apache/kafka/blob/trunk/test-common/test-common-runtime/src/main/java/org/apache/kafka/common/test/TestKitNodes.java#L175

Yeah I'll change it just in case we ever change BROKER_ID_OFFSET.

chia7712

@kevin-wu24 thanks for updates. LGTM

@jsancio Did you get around to reviewing it?

--no-intial-controllers should not assume kraft.version=1

dcc3668

github-actions bot added triage PRs from the community core Kafka Broker kraft small Small PRs labels Sep 17, 2025

jsancio added the ci-approved label Sep 17, 2025

FrankYang0529 requested review from FrankYang0529 and chia7712 September 18, 2025 01:46

refactor kafkaclustertestkit to match semantics of format tool regard…

e0ad6c0

…ing setting kraft version

github-actions bot removed the small Small PRs label Sep 18, 2025

fix build

1720ad7

brandboat reviewed Sep 18, 2025

View reviewed changes

frankvicky reviewed Sep 18, 2025

View reviewed changes

fix test

a905fc8

absence of static voters config determines kraft version at format ti…

e671a39

…me, cleanup of kafka cluster test kit

kevin-wu24 commented Sep 18, 2025

View reviewed changes

kevin-wu24 added 2 commits September 18, 2025 11:29

cleanup

ae21d0e

formatter should not check kraft.version against release version

8917f9d

kevin-wu24 changed the title ~~KAFKA-19719: --no-intial-controllers should not assume kraft.version=1~~ KAFKA-19719: --no-initial-controllers should not assume kraft.version=1 Sep 18, 2025

kevin-wu24 added 2 commits September 18, 2025 12:14

cleanup

9864633

fixing test

951b874

github-actions bot removed the triage PRs from the community label Sep 19, 2025

chia7712 reviewed Sep 19, 2025

View reviewed changes

code review

c99a253

chia7712 approved these changes Sep 21, 2025

View reviewed changes

FrankYang0529 reviewed Sep 21, 2025

View reviewed changes

cleanup

c087a97

jsancio reviewed Sep 22, 2025

View reviewed changes

kevin-wu24 added 3 commits September 22, 2025 10:57

code review and removing duplicate tests

8c0271b

fix error msg

cf4a00e

remove references to setting kraft.version explicitly in docs

f8926de

jsancio reviewed Sep 23, 2025

View reviewed changes

cleanup storage help output

6e23ed8

chia7712 reviewed Sep 24, 2025

View reviewed changes

kevin-wu24 added 3 commits September 24, 2025 08:16

Merge branch 'trunk' into KAFKA-19719

d1e3b65

code review + removing KRaftVersion dependency on MV

14fabc4

fix test

989796a

chia7712 approved these changes Sep 24, 2025

View reviewed changes

		public Formatter setHasDynamicQuorum(boolean staticVotersEmpty) {
		this.hasDynamicQuorum = staticVotersEmpty;

KAFKA-19719: --no-initial-controllers should not assume kraft.version=1 #20551

Are you sure you want to change the base?

KAFKA-19719: --no-initial-controllers should not assume kraft.version=1 #20551

Conversation

kevin-wu24 commented Sep 17, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandboat left a comment

Choose a reason for hiding this comment

Uh oh!

frankvicky left a comment

Choose a reason for hiding this comment

Uh oh!

chia7712 commented Sep 18, 2025

Uh oh!

kevin-wu24 commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevin-wu24 commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chia7712 commented Sep 21, 2025

Uh oh!

FrankYang0529 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsancio left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

kevin-wu24 commented Sep 17, 2025 •

edited by github-actions bot

Loading

kevin-wu24 commented Sep 18, 2025 •

edited

Loading

kevin-wu24 commented Sep 19, 2025 •

edited

Loading