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 2020/10/11 19:51:34 UTC
[GitHub] [kafka] kowshik opened a new pull request #9412: MINOR: Ensure broker advertises features only when versioning system is enabled
kowshik opened a new pull request #9412:
URL: https://github.com/apache/kafka/pull/9412
This is a fix that ensures the broker advertises features to ZK only when versioning system is enabled.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kowshik commented on pull request #9412: MINOR: Ensure broker advertises features only when versioning system is enabled
Posted by GitBox <gi...@apache.org>.
kowshik commented on pull request #9412:
URL: https://github.com/apache/kafka/pull/9412#issuecomment-706761146
@abbccdda @junrao : Please could you have a look at this fix. We have to merge it into trunk as well as the 2.7 release branch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kowshik edited a comment on pull request #9412: MINOR: Ensure broker advertises features only when versioning system is enabled
Posted by GitBox <gi...@apache.org>.
kowshik edited a comment on pull request #9412:
URL: https://github.com/apache/kafka/pull/9412#issuecomment-707241757
@abbccdda Sorry for the confusion. Upon second look, I feel we can abandon this PR containing the fix.
I was looking at the code again, and it seems the requirement is already take care of in the earlier PRs. If IBP >= `KAFKA_2_7_IV0` we set Broker version to 5, which eventually causes features to be advertised in `BrokerIdZNode.encode`. As a result, the logic is already restrictive like the way we would have liked. Here are some pointers to the code:
https://github.com/apache/kafka/blob/fb4f297207ef62f71e4a6d2d0dac75752933043d/core/src/main/scala/kafka/zk/ZkData.scala#L99-L100
https://github.com/apache/kafka/blob/fb4f297207ef62f71e4a6d2d0dac75752933043d/core/src/main/scala/kafka/zk/ZkData.scala#L151-L153
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] abbccdda commented on a change in pull request #9412: MINOR: Ensure broker advertises features only when versioning system is enabled
Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #9412:
URL: https://github.com/apache/kafka/pull/9412#discussion_r503411583
##########
File path: core/src/main/scala/kafka/server/KafkaServer.scala
##########
@@ -494,8 +495,14 @@ class KafkaServer(val config: KafkaConfig, time: Time = Time.SYSTEM, threadNameP
)
val jmxPort = System.getProperty("com.sun.management.jmxremote.port", "-1").toInt
+
+ val supportedFeatures =
Review comment:
Could we add a test coverage for this logic?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kowshik commented on pull request #9412: MINOR: Ensure broker advertises features only when versioning system is enabled
Posted by GitBox <gi...@apache.org>.
kowshik commented on pull request #9412:
URL: https://github.com/apache/kafka/pull/9412#issuecomment-707241757
@abbccdda Sorry for the confusion. Upon second look, I feel we can abandon this PR containing the fix.
I was looking at the code again, and it seems the requirement is already take care of in the earlier PRs. If IBP >= KAFKA_2_7_IV0 we set Broker version to 5, which eventually causes features to be advertised in BrokerIdZNode.encode. As a result, the logic is already restrictive like the way we would have liked. Here are some pointers to the code:
https://github.com/apache/kafka/blob/fb4f297207ef62f71e4a6d2d0dac75752933043d/core/src/main/scala/kafka/zk/ZkData.scala#L99-L100
https://github.com/apache/kafka/blob/fb4f297207ef62f71e4a6d2d0dac75752933043d/core/src/main/scala/kafka/zk/ZkData.scala#L151-L153
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kowshik closed pull request #9412: MINOR: Ensure broker advertises features only when versioning system is enabled
Posted by GitBox <gi...@apache.org>.
kowshik closed pull request #9412:
URL: https://github.com/apache/kafka/pull/9412
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org