MINOR: Fixed documentation on formatting new brokers#21981
MINOR: Fixed documentation on formatting new brokers#21981ppatierno wants to merge 1 commit intoapache:trunkfrom
Conversation
Signed-off-by: Paolo Patierno <ppatierno@live.com>
|
@showuon FYI |
There was a problem hiding this comment.
Thanks for the PR! I had a look, and confirm that the --no-initial-controllers option is used to identify the controller is used for dynamic quorum (i.e. kraft.version=1). In controller role, if any of --no-initial-controllers/--standalone/--initial-controllers is set, a 0-0.checkpoint file will be generated with kraft.version=1 appended. If there is not any of these 3 options set, it'll be formatted with static quorum, and the 0-0.checkpoint will not be generated. So by default, the quorum will be treated as static quorum (kraft.version=0). But for broker role, no matter it's static or dynamic, there will be no 0-0.checkpoint file generated because it is an observer role, not a voter.
About this PR, could we change the wording to let users know that adding --no-initial-controllers or not for brokers doesn't change anything. Because the wording in the PR makes users think the current formatting way is the "correct" way, while the old way is wrong. So, maybe something like this:
# keep the original wording
When provisioning new broker and controller nodes that we want to add to an existing Kafka cluster, use the `kafka-storage.sh format` command with the --no-initial-controllers flag.
$ bin/kafka-storage.sh format --cluster-id <CLUSTER_ID> --config config/server.properties --no-initial-controllers
# add one more paragraph
For the new "broker" nodes, it also works to use `kafka-storage.sh format` command with no additional flags....
$ bin/kafka-storage.sh format --cluster-id <CLUSTER_ID> --config config/server.properties
Something like this. WDYT?
I can change it if you like but it sounds to me like this ... "when formatting a new broker use this flag, but if you don't use it, it will work anyway", so what's the point of saying you can use the flag? |
Hmm... yes, you are right. Let's keep the current change. My only suggestion is that we separate them into 2 sections since they have different commands now:
|
|
Re-reading your additional investigation my only concern is ... was it done in this way because as you said, without the flag, it's formatted as it was in static quorum with kraft.version=0. Does it really work with a broker in static quorum within a cluster where KRaft controllers are working in dynamic quorum? |
I think so because when formatting with |
The current documentation states that when formatting a new broker we should use the
--no-initial-controllersflag while looking at the code it doesn't seem to be necessary.I tried doing that without such a flag. The formatting works and the broker starts up without issue. If we think about that, at storage format time, what the tool gets is the broker config properties file and it can't detected if we are going to use this broker for a new cluster or an existing cluster, so I don't understand why the documentation mentions the usage of the flag for new brokers (which is understandable for the controller instead). Even today, with static quorum, we format brokers without such a flag.
Even looking at the source of
StorageTool, when the--no-initial-controllersflag is missing, the check is done only against controller. For the broker it doesn't care at all. See https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/tools/StorageTool.scala#L162This PR tries to fix the documentation and making it clearer.