You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/06/14 11:15:01 UTC
[GitHub] [kafka] dengziming opened a new pull request, #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
dengziming opened a new pull request, #12294:
URL: https://github.com/apache/kafka/pull/12294
*More detailed description of your change*
Update metadata.version using admin will fail with the following exception:
```
java.util.concurrent.ExecutionException: org.apache.kafka.common.errors.InvalidUpdateVersionException: Invalid update version 7 for feature metadata.version. Controller 3000 does not support this feature.
```
This is because we just return empty supported features in Controller ApiVersionResponse, so `QuorumFeatures.reasonNotSupported` will always assume we don't support this version.
*Summary of testing strategy (including rationale)*
KRaftClusterTest.testUpdateMetadataVersion
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12294:
URL: https://github.com/apache/kafka/pull/12294#issuecomment-1225097010
This is an issue of big influence since it will prevent us from upgrading KRaft cluster, also ping @hachikuji @mumrah
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on a diff in pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
dengziming commented on code in PR #12294:
URL: https://github.com/apache/kafka/pull/12294#discussion_r957969115
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -832,4 +833,31 @@ class KRaftClusterTest {
}
}
+
+ @Test
+ def testUpdateMetadataVersion(): Unit = {
Review Comment:
This test case will fail without this change, can you take a look at this PR @jsancio , I wish this can be merged into 3.3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
cmccabe commented on PR #12294:
URL: https://github.com/apache/kafka/pull/12294#issuecomment-1233288601
So, the reason we didn't notice this bug earlier is that it doesn't affect single-node clusters. And in the multi-node ducktape test that I wrote, the upgrade command was failing but we didn't notice because the error code returned was 0 (success). I will open a PR to fix the kafka-features.sh return code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
cmccabe merged PR #12294:
URL: https://github.com/apache/kafka/pull/12294
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jsancio commented on a diff in pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
jsancio commented on code in PR #12294:
URL: https://github.com/apache/kafka/pull/12294#discussion_r959749618
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -832,4 +833,31 @@ class KRaftClusterTest {
}
}
+
+ @Test
+ def testUpdateMetadataVersion(): Unit = {
Review Comment:
Thanks. I'll mark as a blocker for now but it would be good for @mumrah or @cmccabe to comment and review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12294:
URL: https://github.com/apache/kafka/pull/12294#issuecomment-1190983840
cc @mumrah
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12294:
URL: https://github.com/apache/kafka/pull/12294#issuecomment-1155049348
Hello @cmccabe , in #12207 you moved the validation from `FeatureControlManager.replay` to `FeatureControlManager.updateFeature`, but it's still failing since controller doesn't return right supported versions in ApiVersionResponse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #12294: KAFKA-13990: KRaft controller should return right features in ApiVersionResponse
Posted by GitBox <gi...@apache.org>.
cmccabe commented on PR #12294:
URL: https://github.com/apache/kafka/pull/12294#issuecomment-1233178487
Thanks for finding this, @dengziming . It is a great find and definitely a 3.3 blocker.
Two comments:
* ZK brokers should not expose metadata.version (they should have a supported range from 0-0, aka no supported range)
* I want to get rid of all the "use an empty feature set" factory functions in `ApiVersionsResponse.java`. I made this comment before on a different review, and we left them in, and now we had this major bug. It's time to get rid of all of them. If people want an empty feature set, they can specify that explicitly.
Thanks again for the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org