junrao commented on code in PR #17886:
URL: https://github.com/apache/kafka/pull/17886#discussion_r1854376505
##########
server-common/src/test/java/org/apache/kafka/server/common/FeaturesTest.java:
##########
@@ -115,8 +115,8 @@ public void testDefaultValueAllFeatures(Features feature) {
@ParameterizedTest
@EnumSource(Features.class)
- public void testLatestProductionMapsToLatestMetadataVersion(Features
features) {
- assertEquals(features.latestProduction(),
features.defaultValue(MetadataVersion.LATEST_PRODUCTION));
+ public void testLatestProductionIsNotBehindLatestMetadataVersion(Features
features) {
Review Comment:
We already validate this in Features. Is this test still needed?
##########
server-common/src/main/java/org/apache/kafka/server/common/Features.java:
##########
@@ -177,4 +192,51 @@ public static Features featureFromName(String featureName)
{
public static Map<String, Short> featureImplsToMap(List<FeatureVersion>
features) {
return
features.stream().collect(Collectors.toMap(FeatureVersion::featureName,
FeatureVersion::featureLevel));
}
+
+ public boolean isProductionReady(short featureVersion) {
+ return featureVersion <= latestProduction();
+ }
+
+ /**
+ * The method ensures that the following statements are met:
+ * 1. The latest production value >= the default value.
+ * 2. The dependencies of the latest production value <= their latest
production values.
+ * 3. The dependencies of the default value <= their default values.
+ */
+ public static void validateDefaultValueAndLatestProductionValue(Features
feature) {
+ FeatureVersion defaultVersion =
feature.defaultVersion(MetadataVersion.LATEST_PRODUCTION);
+ if (feature.latestProduction() < defaultVersion.featureLevel()) {
+ throw new IllegalArgumentException("The latest production value
should be no smaller than the default value.");
Review Comment:
Could we include the feature name and level in the exception? Ditto below.
##########
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##########
@@ -32,6 +32,9 @@ public enum TestFeatureVersion implements FeatureVersion {
public static final String FEATURE_NAME = "test.feature.version";
+ public static final TestFeatureVersion LATEST_PRODUCTION =
+ MetadataVersion.latestProduction() == MetadataVersion.latestTesting()
? TEST_2 : TEST_1;
Review Comment:
The comment above says "TEST_2 is not yet released ". We probably want to
adjust it to make it more accurate.
##########
server-common/src/main/java/org/apache/kafka/server/common/Features.java:
##########
@@ -142,25 +153,29 @@ public static void validateVersion(FeatureVersion
feature, Map<String, Short> fe
}
/**
- * A method to return the default (latest production) level of a feature
based on the metadata version provided.
+ * A method to return the default (latest production) version of a feature
based on the metadata version provided.
*
* Every time a new feature is added, it should create a mapping from
metadata version to feature version
- * with {@link FeatureVersion#bootstrapMetadataVersion()}. When the
feature version is production ready, the metadata
- * version should be made production ready as well.
+ * with {@link FeatureVersion#bootstrapMetadataVersion()}. The feature
version should be marked as production ready
+ * before the metadata version is made production ready.
*
* @param metadataVersion the metadata version we want to use to set the
default.
- * @return the default version level given the feature and provided
metadata version
+ * @return the default version given the feature and provided metadata
version
*/
- public short defaultValue(MetadataVersion metadataVersion) {
- short level = 0;
+ public FeatureVersion defaultVersion(MetadataVersion metadataVersion) {
+ FeatureVersion version = featureVersions[0];
for (Iterator<FeatureVersion> it =
Arrays.stream(featureVersions).iterator(); it.hasNext(); ) {
FeatureVersion feature = it.next();
if (feature.bootstrapMetadataVersion().isLessThan(metadataVersion)
|| feature.bootstrapMetadataVersion().equals(metadataVersion))
- level = feature.featureLevel();
+ version = feature;
else
- return level;
+ return version;
}
- return level;
+ return version;
+ }
+
+ public short defaultValue(MetadataVersion metadataVersion) {
Review Comment:
defaultValue => defaultLevel ?
--
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]
