HDDS-14735. ComponentVersions should not be compared using int values.

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java

@@ -285,8 +285,8 @@ private boolean allDataNodesSupportPiggybacking() {
     // that supports PutBlock piggybacking.
     for (DatanodeDetails dn : pipeline.getNodes()) {
       LOG.debug("dn = {}, version = {}", dn, dn.getCurrentVersion());
-      if (dn.getCurrentVersion() <
-              COMBINED_PUTBLOCK_WRITECHUNK_RPC.serialize()) {
+      if (!COMBINED_PUTBLOCK_WRITECHUNK_RPC.isSupportedBy(
+          dn.getCurrentVersion())) {
         return false;
       }
     }

hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockInputStreamFactoryImpl.java

@@ -103,7 +103,7 @@ private boolean allDataNodesSupportStreamBlock(Pipeline pipeline) {
     // return true only if all DataNodes in the pipeline are on a version
     // that supports for reading a block by streaming chunks..
     for (DatanodeDetails dn : pipeline.getNodes()) {
-      if (dn.getCurrentVersion() < STREAM_BLOCK_SUPPORT.serialize()) {
+      if (!STREAM_BLOCK_SUPPORT.isSupportedBy(dn.getCurrentVersion())) {
         return false;
       }
     }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ComponentVersion.java

@@ -24,13 +24,24 @@
  * Base type for component version enums.
  */
 public interface ComponentVersion {
+  /**
+   * @return The serialized representation of this version. This is an opaque value which should not be checked or
+   * compared directly.
+   */
   int serialize();
 
   /**
    * @return the description of the version enum value.
    */
   String description();
 
+  /**
+   * Deserializes a ComponentVersion and checks if its feature set is supported by the current ComponentVersion.
+   *
+   * @return true if this version supports the features of otherVersion. False otherwise.
+   */
+  boolean isSupportedBy(int serializedVersion);
+
   default Optional<? extends UpgradeAction> action() {
     return Optional.empty();
   }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HDDSVersion.java

@@ -39,10 +39,9 @@ public enum HDDSVersion implements ComponentVersion {
   FUTURE_VERSION(-1, "Used internally in the client when the server side is "
       + " newer and an unknown server version has arrived to the client.");
 
-  public static final HDDSVersion CURRENT = latest();
-  public static final int CURRENT_VERSION = CURRENT.version;
+  public static final HDDSVersion SOFTWARE_VERSION = latest();
 
-  private static final Map<Integer, HDDSVersion> BY_PROTO_VALUE =
+  private static final Map<Integer, HDDSVersion> BY_VALUE =
       Arrays.stream(values())
           .collect(toMap(HDDSVersion::serialize, identity()));
 
@@ -64,17 +63,26 @@ public int serialize() {
     return version;
   }
 
+  public static HDDSVersion deserialize(int value) {
+    return BY_VALUE.getOrDefault(value, FUTURE_VERSION);
+  }
+
   @Override
-  public String toString() {
-    return name() + " (" + serialize() + ")";
+  public boolean isSupportedBy(int serializedVersion) {
+    // In order for the other serialized version to support this version's features,
+    // the other version must be equal or larger to this version.
+    return deserialize(serializedVersion).compareTo(this) >= 0;
   }
 
-  public static HDDSVersion fromProtoValue(int value) {
-    return BY_PROTO_VALUE.getOrDefault(value, FUTURE_VERSION);
+  @Override
+  public String toString() {
+    return name() + " (" + serialize() + ")";
   }
 
   private static HDDSVersion latest() {
     HDDSVersion[] versions = HDDSVersion.values();
+    // The last entry in the array will be `FUTURE_VERSION`. We want the entry prior to this which defines the latest
+    // version in the software.
     return versions[versions.length - 2];
   }
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java

@@ -517,7 +517,7 @@ public static DatanodeDetails getFromProtoBuf(
    */
   @JsonIgnore
   public HddsProtos.DatanodeDetailsProto getProtoBufMessage() {
-    return toProto(ClientVersion.CURRENT_VERSION);
+    return toProto(ClientVersion.CURRENT.serialize());
   }
 
   public HddsProtos.DatanodeDetailsProto toProto(int clientVersion) {
@@ -572,8 +572,7 @@ public HddsProtos.DatanodeDetailsProto.Builder toProtoBuilder(
     builder.setPersistedOpStateExpiry(persistedOpStateExpiryEpochSec);
 
     final boolean handlesUnknownPorts =
-        ClientVersion.fromProtoValue(clientVersion)
-        .compareTo(VERSION_HANDLES_UNKNOWN_DN_PORTS) >= 0;
+        VERSION_HANDLES_UNKNOWN_DN_PORTS.isSupportedBy(clientVersion);
     final int requestedPortCount = filterPorts.size();
     final boolean maySkip = requestedPortCount > 0;
     for (Port port : ports) {
@@ -727,7 +726,7 @@ public static final class Builder {
     private HddsProtos.NodeOperationalState persistedOpState;
     private long persistedOpStateExpiryEpochSec = 0;
     private int initialVersion;
-    private int currentVersion = HDDSVersion.CURRENT_VERSION;
+    private int currentVersion = HDDSVersion.SOFTWARE_VERSION.serialize();
 
     /**
      * Default private constructor. To create Builder instance use

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java

@@ -67,7 +67,7 @@ public final class Pipeline {
   private static final Codec<Pipeline> CODEC = new DelegatedCodec<>(
       Proto2Codec.get(HddsProtos.Pipeline.getDefaultInstance()),
       Pipeline::getFromProtobufSetCreationTimestamp,
-      p -> p.getProtobufMessage(ClientVersion.CURRENT_VERSION),
+      p -> p.getProtobufMessage(ClientVersion.CURRENT.serialize()),
       Pipeline.class,
       DelegatedCodec.CopyType.UNSUPPORTED);
 

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/ClientVersion.java

@@ -21,7 +21,6 @@
 import static java.util.stream.Collectors.toMap;
 
 import java.util.Arrays;
-import java.util.Comparator;
 import java.util.Map;
 import org.apache.hadoop.hdds.ComponentVersion;
 
@@ -46,9 +45,8 @@ public enum ClientVersion implements ComponentVersion {
       + " unknown client version has arrived from the client.");
 
   public static final ClientVersion CURRENT = latest();
-  public static final int CURRENT_VERSION = CURRENT.version;
 
-  private static final Map<Integer, ClientVersion> BY_PROTO_VALUE =
+  private static final Map<Integer, ClientVersion> BY_VALUE =
       Arrays.stream(values())
           .collect(toMap(ClientVersion::serialize, identity()));
 
@@ -70,18 +68,26 @@ public int serialize() {
     return version;
   }
 
+  public static ClientVersion deserialize(int value) {
+    return BY_VALUE.getOrDefault(value, FUTURE_VERSION);
+  }
+
   @Override
-  public String toString() {
-    return name() + " (" + serialize() + ")";
+  public boolean isSupportedBy(int serializedVersion) {
+    // In order for the other serialized version to support this version's features,
+    // the other version must be equal or larger to this version.
+    return deserialize(serializedVersion).compareTo(this) >= 0;
   }
 
-  public static ClientVersion fromProtoValue(int value) {
-    return BY_PROTO_VALUE.getOrDefault(value, FUTURE_VERSION);
+  @Override
+  public String toString() {
+    return name() + " (" + serialize() + ")";
   }
 
   private static ClientVersion latest() {
-    return Arrays.stream(ClientVersion.values())
-        .max(Comparator.comparingInt(ComponentVersion::serialize)).orElse(null);
+    ClientVersion[] versions = ClientVersion.values();
+    // The last entry in the array will be `FUTURE_VERSION`. We want the entry prior to this which defines the latest
+    // version in the software.
+    return versions[versions.length - 2];
   }
-
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java

@@ -58,9 +58,9 @@ public enum OzoneManagerVersion implements ComponentVersion {
   FUTURE_VERSION(-1, "Used internally in the client when the server side is "
       + " newer and an unknown server version has arrived to the client.");
 
-  public static final OzoneManagerVersion CURRENT = latest();
+  public static final OzoneManagerVersion SOFTWARE_VERSION = latest();
 
-  private static final Map<Integer, OzoneManagerVersion> BY_PROTO_VALUE =
+  private static final Map<Integer, OzoneManagerVersion> BY_VALUE =
       Arrays.stream(values())
           .collect(toMap(OzoneManagerVersion::serialize, identity()));
 
@@ -82,17 +82,26 @@ public int serialize() {
     return version;
   }
 
+  public static OzoneManagerVersion deserialize(int value) {
+    return BY_VALUE.getOrDefault(value, FUTURE_VERSION);
+  }
+
   @Override
-  public String toString() {
-    return name() + " (" + serialize() + ")";
+  public boolean isSupportedBy(int serializedVersion) {
+    // In order for the other serialized version to support this version's features,
+    // the other version must be equal or larger to this version.
+    return deserialize(serializedVersion).compareTo(this) >= 0;
   }
 
-  public static OzoneManagerVersion fromProtoValue(int value) {
-    return BY_PROTO_VALUE.getOrDefault(value, FUTURE_VERSION);
+  @Override
+  public String toString() {
+    return name() + " (" + serialize() + ")";
   }
 
   private static OzoneManagerVersion latest() {
     OzoneManagerVersion[] versions = OzoneManagerVersion.values();
+    // The last entry in the array will be `FUTURE_VERSION`. We want the entry prior to this which defines the latest
+    // version in the software.
     return versions[versions.length - 2];
   }
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/LayoutFeature.java

@@ -29,4 +29,12 @@ public interface LayoutFeature extends ComponentVersion {
   default int serialize() {
     return this.layoutVersion();
   }
+
+  @Override
+  default boolean isSupportedBy(int serializedVersion) {
+    // In order for the other serialized version to support this version's features,
+    // the other version must be equal or larger to this version.
+    // We can compare the values directly since there is no FUTURE_VERSION for layout features.
+    return serializedVersion >= layoutVersion();
+  }
 }

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestComponentVersionInvariants.java

@@ -18,11 +18,13 @@
 package org.apache.hadoop.hdds;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import java.util.stream.Stream;
 import org.apache.hadoop.ozone.ClientVersion;
 import org.apache.hadoop.ozone.OzoneManagerVersion;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -53,8 +55,7 @@ public static Stream<Arguments> values() {
   // FUTURE_VERSION is the latest
   @ParameterizedTest
   @MethodSource("values")
-  public void testFutureVersionHasTheHighestOrdinal(
-      ComponentVersion[] values, ComponentVersion defaultValue,
+  public void testFutureVersionHasTheHighestOrdinal(ComponentVersion[] values, ComponentVersion defaultValue,
       ComponentVersion futureValue) {
 
     assertEquals(values[values.length - 1], futureValue);
@@ -63,8 +64,7 @@ public void testFutureVersionHasTheHighestOrdinal(
   // FUTURE_VERSION's internal version id is -1
   @ParameterizedTest
   @MethodSource("values")
-  public void testFuturVersionHasMinusOneAsProtoRepresentation(
-      ComponentVersion[] values, ComponentVersion defaultValue,
+  public void testFutureVersionSerializesToMinusOne(ComponentVersion[] values, ComponentVersion defaultValue,
       ComponentVersion futureValue) {
     assertEquals(-1, futureValue.serialize());
 
@@ -73,17 +73,15 @@ public void testFuturVersionHasMinusOneAsProtoRepresentation(
   // DEFAULT_VERSION's internal version id is 0
   @ParameterizedTest
   @MethodSource("values")
-  public void testDefaultVersionHasZeroAsProtoRepresentation(
-      ComponentVersion[] values, ComponentVersion defaultValue,
+  public void testDefaultVersionSerializesToZero(ComponentVersion[] values, ComponentVersion defaultValue,
       ComponentVersion futureValue) {
     assertEquals(0, defaultValue.serialize());
   }
 
   // versions are increasing monotonically by one
   @ParameterizedTest
   @MethodSource("values")
-  public void testAssignedProtoRepresentations(
-      ComponentVersion[] values, ComponentVersion defaultValue,
+  public void testSerializedValuesAreMonotonic(ComponentVersion[] values, ComponentVersion defaultValue,
       ComponentVersion futureValue) {
     int startValue = defaultValue.serialize();
     // we skip the future version at the last position
@@ -92,4 +90,59 @@ public void testAssignedProtoRepresentations(
     }
     assertEquals(values.length, ++startValue);
   }
+
+  @ParameterizedTest
+  @MethodSource("values")
+  public void testVersionIsSupportedByItself(ComponentVersion[] values, ComponentVersion defaultValue,
+      ComponentVersion futureValue) {
+    for (ComponentVersion value : values) {
+      assertTrue(value.isSupportedBy(value.serialize()));
+    }
+  }
+
+  @ParameterizedTest
+  @MethodSource("values")
+  public void testOnlyEqualOrHigherVersionsCanSupportAFeature(ComponentVersion[] values, ComponentVersion defaultValue,
+      ComponentVersion futureValue) {
+    int knownVersionCount = values.length - 1;
+    for (int featureIndex = 0; featureIndex < knownVersionCount; featureIndex++) {
+      ComponentVersion requiredFeature = values[featureIndex];
+      for (int providerIndex = 0; providerIndex < knownVersionCount; providerIndex++) {
+        ComponentVersion provider = values[providerIndex];
+        boolean expected = providerIndex >= featureIndex;
+        assertEquals(expected, requiredFeature.isSupportedBy(provider.serialize()));
+      }
+    }
+  }
+
+  @ParameterizedTest
+  @MethodSource("values")
+  public void testFutureVersionSupportsAllKnownVersions(ComponentVersion[] values, ComponentVersion defaultValue,
+      ComponentVersion futureValue) {
+    int unknownFutureVersion = Integer.MAX_VALUE;
+    for (ComponentVersion requiredFeature : values) {
+      assertTrue(requiredFeature.isSupportedBy(unknownFutureVersion));
+    }
+  }
+
+  @Test
+  public void testHDDSVersionSerDes() {
+    for (HDDSVersion version: HDDSVersion.values()) {
+      assertEquals(version, HDDSVersion.deserialize(version.serialize()));
+    }
+  }
+
+  @Test
+  public void testOMVersionSerDes() {
+    for (OzoneManagerVersion version: OzoneManagerVersion.values()) {
+      assertEquals(version, OzoneManagerVersion.deserialize(version.serialize()));
+    }
+  }
+
+  @Test
+  public void testClientVersionSerDes() {
+    for (ClientVersion version: ClientVersion.values()) {
+      assertEquals(version, ClientVersion.deserialize(version.serialize()));
+    }
+  }
 }

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/protocol/TestDatanodeDetails.java

@@ -77,13 +77,15 @@ public void testNewBuilderCurrentVersion() {
         dn.toProtoBuilder(DEFAULT_VERSION.serialize(), requiredPorts);
     protoBuilder.clearCurrentVersion();
     DatanodeDetails dn2 = DatanodeDetails.newBuilder(protoBuilder.build()).build();
-    assertEquals(HDDSVersion.SEPARATE_RATIS_PORTS_AVAILABLE.serialize(), dn2.getCurrentVersion());
+    assertEquals(HDDSVersion.SEPARATE_RATIS_PORTS_AVAILABLE,
+        HDDSVersion.deserialize(dn2.getCurrentVersion()));
 
     // test that if the current version is set, it is used
     protoBuilder =
         dn.toProtoBuilder(DEFAULT_VERSION.serialize(), requiredPorts);
     DatanodeDetails dn3 = DatanodeDetails.newBuilder(protoBuilder.build()).build();
-    assertEquals(HDDSVersion.CURRENT.serialize(), dn3.getCurrentVersion());
+    assertEquals(HDDSVersion.SOFTWARE_VERSION,
+        HDDSVersion.deserialize(dn3.getCurrentVersion()));
   }
 
   public static void assertPorts(HddsProtos.DatanodeDetailsProto dn,

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java

@@ -816,13 +816,13 @@ private String reconfigScmNodes(String value) {
    * Returns the initial version of the datanode.
    */
   private int getInitialVersion() {
-    return conf.getInt(TESTING_DATANODE_VERSION_INITIAL, HDDSVersion.CURRENT_VERSION);
+    return conf.getInt(TESTING_DATANODE_VERSION_INITIAL, HDDSVersion.SOFTWARE_VERSION.serialize());
   }
 
   /**
    * Returns the current version of the datanode.
    */
   private int getCurrentVersion() {
-    return conf.getInt(TESTING_DATANODE_VERSION_CURRENT, HDDSVersion.CURRENT_VERSION);
+    return conf.getInt(TESTING_DATANODE_VERSION_CURRENT, HDDSVersion.SOFTWARE_VERSION.serialize());
   }
 }