Re: [PR] KAFKA-17011: SupportedFeatures.MinVersion incorrectly blocks v0 (3.8) [kafka]
junrao commented on code in PR #16420:
URL: https://github.com/apache/kafka/pull/16420#discussion_r1653240450
##
clients/src/main/java/org/apache/kafka/common/requests/ApiVersionsResponse.java:
##
@@ -258,11 +259,22 @@ private static ApiVersionsResponseData
createApiVersionsResponseData(
final long finalizedFeaturesEpoch,
final boolean zkMigrationEnabled
) {
+Features backwardsCompatibleFeatures =
Features.supportedFeatures(latestSupportedFeatures.features().entrySet()
+.stream().filter(entry -> {
+SupportedVersionRange supportedVersionRange = entry.getValue();
+return supportedVersionRange.min() != 0;
Review Comment:
Perhaps it's useful to add a comment on why we are filtering version 0
features.
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-17011: SupportedFeatures.MinVersion incorrectly blocks v0 (3.8) [kafka]
jolshan commented on PR #16420: URL: https://github.com/apache/kafka/pull/16420#issuecomment-2189721182 Thanks @junrao! Done -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-17011: SupportedFeatures.MinVersion incorrectly blocks v0 (3.8) [kafka]
jolshan commented on PR #16420: URL: https://github.com/apache/kafka/pull/16420#issuecomment-2189619505 I will merge in the next hour or so unless there are further concerns. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-17011: SupportedFeatures.MinVersion incorrectly blocks v0 (3.8) [kafka]
chia7712 commented on PR #16420:
URL: https://github.com/apache/kafka/pull/16420#issuecomment-2186476992
BTW, there is a description in KIP-584
> Each broker’s supported dictionary of {feature → version range} will be
defined in the broker code. For each supported feature, the supported version
range is defined by a min_version (an int64 starting always from 1) and
max_version (an int64 >=1 and >= min_version).
not sure why #15671 changed the start version from 1 to 0
https://github.com/apache/kafka/pull/15671/files#r1576879728
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-17011: SupportedFeatures.MinVersion incorrectly blocks v0 (3.8) [kafka]
chia7712 commented on code in PR #16420:
URL: https://github.com/apache/kafka/pull/16420#discussion_r1650736148
##
clients/src/main/java/org/apache/kafka/common/requests/ApiVersionsResponse.java:
##
@@ -258,11 +259,22 @@ private static ApiVersionsResponseData
createApiVersionsResponseData(
final long finalizedFeaturesEpoch,
final boolean zkMigrationEnabled
) {
+Features backwardsCompatibleFeatures =
Features.supportedFeatures(latestSupportedFeatures.features().entrySet()
+.stream().filter(entry -> {
+SupportedVersionRange supportedVersionRange = entry.getValue();
+return supportedVersionRange.min() != 0;
Review Comment:
not sure whether I fail to catch your description.
> so for now we will change 0 to 1 in the response in order to be backwards
compatible.
the code looks like it gets rid of "0" instead of changing to from 0 to 1.
##
clients/src/main/java/org/apache/kafka/common/requests/ApiVersionsResponse.java:
##
@@ -258,11 +259,22 @@ private static ApiVersionsResponseData
createApiVersionsResponseData(
final long finalizedFeaturesEpoch,
final boolean zkMigrationEnabled
) {
+Features backwardsCompatibleFeatures =
Features.supportedFeatures(latestSupportedFeatures.features().entrySet()
+.stream().filter(entry -> {
+SupportedVersionRange supportedVersionRange = entry.getValue();
+return supportedVersionRange.min() != 0;
Review Comment:
For another, does this PR mean `Admin#describeFeatures` can't see the
feature "group.version" from the broker running in 3.8.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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-17011: SupportedFeatures.MinVersion incorrectly blocks v0 (3.8) [kafka]
jolshan commented on PR #16420: URL: https://github.com/apache/kafka/pull/16420#issuecomment-2183475015 Ran the kraft upgrade tests and they passed with this change 👍 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
