KAFKA-19719: --no-initial-controllers should not assume kraft.version=1#20595
Closed
kevin-wu24 wants to merge 1 commit into
Closed
KAFKA-19719: --no-initial-controllers should not assume kraft.version=1#20595kevin-wu24 wants to merge 1 commit into
kevin-wu24 wants to merge 1 commit into
Conversation
…=1 (apache#20551) 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 defined by 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>, José Armando García Sancio <jsancio@apache.org> Conflicts: core/src/main/scala/kafka/tools/StorageTool.scala Minor conflicts. core/src/test/java/kafka/server/ReconfigurableQuorumIntegrationTest.java Revert integration test changes to simplify PR. Some tests are not valid in 4.1. docs/ops.html Keep new docs changes. metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java Minor conflicts. Keep new tests. test-common/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java Revert integration test changes to simplify PR.
Contributor
Author
|
I messed up the cherry-pick. I need to keep the integration test changes, otherwise the existing tests break. I'm going to pick this again in a new PR @frankvicky. |
Contributor
Author
|
here's the new PR link: @frankvicky #20604 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 defined by 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.
Reviewers: TengYao Chi kitingiao@gmail.com, Kuan-Po Tseng
brandboat@gmail.com, Chia-Ping Tsai chia7712@gmail.com, José
Armando García Sancio jsancio@apache.org
Conflicts:
core/src/main/scala/kafka/tools/StorageTool.scala
Minor conflicts.
core/src/test/java/kafka/server/ReconfigurableQuorumIntegrationTest.java
Revert integration test changes to simplify PR. Some tests are not valid in 4.1.
docs/ops.html
Keep new docs changes.
metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java
Minor conflicts. Keep new tests.
test-common/test-common-runtime/src/main/java/org/apache/kafka/common/test/KafkaClusterTestKit.java
Revert integration test changes to simplify PR.