jolshan commented on code in PR #15685:
URL: https://github.com/apache/kafka/pull/15685#discussion_r1603398767
##########
metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java:
##########
@@ -459,18 +459,20 @@ BrokerFeature processRegistrationFeature(
FinalizedControllerFeatures finalizedFeatures,
BrokerRegistrationRequestData.Feature feature
) {
- Optional<Short> finalized = finalizedFeatures.get(feature.name());
- if (finalized.isPresent()) {
- if (!VersionRange.of(feature.minSupportedVersion(),
feature.maxSupportedVersion()).contains(finalized.get())) {
- throw new UnsupportedVersionException("Unable to register
because the broker " +
- "does not support version " + finalized.get() + " of " +
feature.name() +
- ". It wants a version between " +
feature.minSupportedVersion() + " and " +
- feature.maxSupportedVersion() + ", inclusive.");
- }
- } else {
- log.warn("Broker {} registered with feature {} that is unknown to
the controller",
+ int defaultVersion =
feature.name().equals(MetadataVersion.FEATURE_NAME) ? 1 : 0; // The default
value for MetadataVersion is 1 not 0.
+ short finalized = finalizedFeatures.getOrDefault(feature.name(),
(short) defaultVersion);
+ if (!VersionRange.of(feature.minSupportedVersion(),
feature.maxSupportedVersion()).contains(finalized)) {
+ throw new UnsupportedVersionException("Unable to register because
the broker " +
+ "does not support version " + finalized + " of " +
feature.name() +
+ ". It wants a version between " +
feature.minSupportedVersion() + " and " +
+ feature.maxSupportedVersion() + ", inclusive.");
+ }
+ // A feature is not found in the finalizedFeature map if it is unknown
to the controller or set to 0 (feature not enabled).
+ // As more features roll out, it may be common to leave a feature
disabled, so this log is debug level in the case
+ // an intended feature is not being set.
+ if (finalized == 0)
+ log.debug("Broker {} registered with feature {} that is either
unknown or version 0 on the controller",
Review Comment:
It's really hard to differentiate between disabled and unknown because the
protocol when setting a feature to 0 is to remove it. You will not have a
record when you set it to 0 because of this.
--
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]
