frankvicky commented on code in PR #18593:
URL: https://github.com/apache/kafka/pull/18593#discussion_r1924620570
##########
server-common/src/main/java/org/apache/kafka/server/common/FinalizedFeatures.java:
##########
@@ -27,25 +27,19 @@ public final class FinalizedFeatures {
private final long finalizedFeaturesEpoch;
public static FinalizedFeatures fromKRaftVersion(MetadataVersion version) {
- return new FinalizedFeatures(version, Collections.emptyMap(), -1,
true);
+ return new FinalizedFeatures(version, Collections.emptyMap(), -1);
}
public FinalizedFeatures(
MetadataVersion metadataVersion,
Map<String, Short> finalizedFeatures,
- long finalizedFeaturesEpoch,
- boolean kraftMode
+ long finalizedFeaturesEpoch
) {
- this.metadataVersion = metadataVersion;
+ this.metadataVersion = Objects.requireNonNull(metadataVersion);
this.finalizedFeatures = new HashMap<>(finalizedFeatures);
this.finalizedFeaturesEpoch = finalizedFeaturesEpoch;
- // In KRaft mode, we always include the metadata version in the
features map.
- // In ZK mode, we never include it.
- if (kraftMode) {
- this.finalizedFeatures.put(MetadataVersion.FEATURE_NAME,
metadataVersion.featureLevel());
- } else {
- this.finalizedFeatures.remove(MetadataVersion.FEATURE_NAME);
- }
+ // Intentionally include metadata version in features map for version
consistency
Review Comment:
Hi @ijuma,
Thanks for the review.
Previously, I simply deleted `// In KRaft mode` from the comment, but I
realized this could be confusing. I'm wondering if we should delete the entire
comment instead?
The original comment was to highlight the difference between zk and kraft
behavior. Now that ZK is gone, do we still need this comment?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
