jsancio commented on code in PR #21122:
URL: https://github.com/apache/kafka/pull/21122#discussion_r3324463238
##########
server-common/src/main/java/org/apache/kafka/server/common/FinalizedFeatures.java:
##########
@@ -19,28 +19,128 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
-public record FinalizedFeatures(
- MetadataVersion metadataVersion,
- Map<String, Short> finalizedFeatures,
- long finalizedFeaturesEpoch
-) {
- public static FinalizedFeatures fromKRaftVersion(MetadataVersion version) {
- return new FinalizedFeatures(version, Map.of(), -1);
- }
+/**
+ * Represents the finalized feature levels for a Kafka cluster.
+ * <p>
+ * This class can be in one of three states:
+ * <ul>
+ * <li>Unknown - the metadata version has not been committed yet, e.g.
before a quorum is
+ * formed. Used as the initial state in {@code FeaturesPublisher}. (use
{@link #unknown()})</li>
+ * <li>Metadata version only - the metadata version is known but no
additional features or
+ * epoch have been set. Used when only the metadata version needs to be
represented
+ * without a full set of finalized features. (use {@link
#fromKRaftVersion(MetadataVersion)})</li>
+ * <li>Full features - metadata version, features map, and epoch are all
known. Used after
+ * the controller has committed feature records. (use {@link
#of(MetadataVersion, Map, long)})</li>
+ * </ul>
+ */
+public final class FinalizedFeatures {
+ private static final FinalizedFeatures UNKNOWN = new
FinalizedFeatures(Optional.empty(), Map.of(), -1);
+
+ private final Optional<MetadataVersion> metadataVersion;
+ private final Map<String, Short> finalizedFeatures;
+ private final long finalizedFeaturesEpoch;
- public FinalizedFeatures(
- MetadataVersion metadataVersion,
+ private FinalizedFeatures(
+ Optional<MetadataVersion> metadataVersion,
Map<String, Short> finalizedFeatures,
long finalizedFeaturesEpoch
) {
- this.metadataVersion = Objects.requireNonNull(metadataVersion);
+ this.metadataVersion = metadataVersion;
this.finalizedFeatures = new HashMap<>(finalizedFeatures);
this.finalizedFeaturesEpoch = finalizedFeaturesEpoch;
- this.finalizedFeatures.put(MetadataVersion.FEATURE_NAME,
metadataVersion.featureLevel());
+ metadataVersion.ifPresent(mv ->
+ this.finalizedFeatures.put(MetadataVersion.FEATURE_NAME,
mv.featureLevel()));
+ }
+
+ /**
+ * Returns a sentinel value representing unknown finalized features.
+ *
+ * @return the unknown finalized features instance
+ */
+ public static FinalizedFeatures unknown() {
+ return UNKNOWN;
+ }
+
+ /**
+ * Creates a new instance from the given KRaft metadata version.
+ *
+ * @param version the metadata version
+ * @return a new FinalizedFeatures instance
+ * @throws NullPointerException if version is null
+ */
+ public static FinalizedFeatures fromKRaftVersion(MetadataVersion version) {
Review Comment:
I missed this. This should be fromMetadataVersion. We now have a KRaft
version feature (kraft.version) it will be confusing to use to see kraft
version method name but the metadata version (metadata.version) is passed.
--
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]