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 2021/07/27 21:32:05 UTC
[GitHub] [kafka] niket-goel opened a new pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
niket-goel opened a new pull request #11135:
URL: https://github.com/apache/kafka/pull/11135
This PR is WIP and the test added does not work yet.
--
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] niket-goel commented on a change in pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
niket-goel commented on a change in pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#discussion_r677815515
##########
File path: clients/src/main/resources/common/message/MetadataRequest.json
##########
@@ -16,9 +16,9 @@
{
"apiKey": 3,
"type": "request",
- "listeners": ["zkBroker", "broker", "controller"],
+ "listeners": ["zkBroker", "broker"],
"name": "MetadataRequest",
- "validVersions": "0-11",
+ "validVersions": "0-12",
Review comment:
I am not sure if just modifying the valid versions bumps the version number (or if we want to bump the version at all)
##########
File path: core/src/test/scala/unit/kafka/server/ControllerApisTest.scala
##########
@@ -145,6 +145,14 @@ class ControllerApisTest {
authorizer
}
+ @Test
+ def testHanldleMetadata(): Unit = {
+ val caught = assertThrows(classOf[ApiException], () => createControllerApis(
+ None, new MockController.Builder().build()).
Review comment:
I think I might have found an NPE. createControllerApis allows for Option[authorizer], however fails with an NPE if None is passed as an authorizer.
--
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] hachikuji merged pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
hachikuji merged pull request #11135:
URL: https://github.com/apache/kafka/pull/11135
--
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] hachikuji commented on a change in pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#discussion_r677818654
##########
File path: clients/src/main/resources/common/message/MetadataRequest.json
##########
@@ -16,9 +16,9 @@
{
"apiKey": 3,
"type": "request",
- "listeners": ["zkBroker", "broker", "controller"],
+ "listeners": ["zkBroker", "broker"],
"name": "MetadataRequest",
- "validVersions": "0-11",
+ "validVersions": "0-12",
Review comment:
We shouldn't need to bump the version here if all we're changing is `listeners`.
--
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] hachikuji commented on pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#issuecomment-888469413
I filed https://issues.apache.org/jira/browse/KAFKA-13146 as a follow-up for after 3.0.
--
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] niket-goel commented on a change in pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
niket-goel commented on a change in pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#discussion_r677871792
##########
File path: core/src/test/scala/unit/kafka/server/ControllerApisTest.scala
##########
@@ -145,6 +145,14 @@ class ControllerApisTest {
authorizer
}
+ @Test
+ def testHanldleMetadata(): Unit = {
+ val caught = assertThrows(classOf[ApiException], () => createControllerApis(
+ None, new MockController.Builder().build()).
Review comment:
Was chasing a phantom. This was not an issue. Incorrect implementation on my part.
--
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] niket-goel commented on a change in pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
niket-goel commented on a change in pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#discussion_r677820374
##########
File path: core/src/main/scala/kafka/server/ControllerApis.scala
##########
@@ -151,6 +150,11 @@ class ControllerApis(val requestChannel: RequestChannel,
handleRaftRequest(request, response => new FetchSnapshotResponse(response.asInstanceOf[FetchSnapshotResponseData]))
}
+ // This API is not wired in yet as the controller does not implement the
+ // metadata completely.
+ // Leaving the code in to be completed and wired in the future
+ //
+ // See https://issues.apache.org/jira/browse/KAFKA-13143 for details
Review comment:
Will remove the 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] hachikuji commented on a change in pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#discussion_r677819183
##########
File path: core/src/main/scala/kafka/server/ControllerApis.scala
##########
@@ -151,6 +150,11 @@ class ControllerApis(val requestChannel: RequestChannel,
handleRaftRequest(request, response => new FetchSnapshotResponse(response.asInstanceOf[FetchSnapshotResponseData]))
}
+ // This API is not wired in yet as the controller does not implement the
+ // metadata completely.
+ // Leaving the code in to be completed and wired in the future
+ //
+ // See https://issues.apache.org/jira/browse/KAFKA-13143 for details
Review comment:
My preference would be to remove the handler completely. We can always bring it back in the future.
--
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] niket-goel commented on a change in pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
niket-goel commented on a change in pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#discussion_r677821728
##########
File path: clients/src/main/resources/common/message/MetadataRequest.json
##########
@@ -16,9 +16,9 @@
{
"apiKey": 3,
"type": "request",
- "listeners": ["zkBroker", "broker", "controller"],
+ "listeners": ["zkBroker", "broker"],
"name": "MetadataRequest",
- "validVersions": "0-11",
+ "validVersions": "0-12",
Review comment:
Makes sense. Will remove the version bump.
--
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] niket-goel commented on pull request #11135: KAFKA-13143 Remove HandleMetadata from ControllerAPis as metadata is not completely implemented on KRaft controllers
Posted by GitBox <gi...@apache.org>.
niket-goel commented on pull request #11135:
URL: https://github.com/apache/kafka/pull/11135#issuecomment-887915977
@jsancio
Without the change (as per Jason's description in the JIRA https://issues.apache.org/jira/browse/KAFKA-13143), the API would return an empty list of topics making the user think that they had no topics.
With the change the describe and list topic APIs timeout on the controller endpoint (same as create_topic).
--
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