HDDS-12621. Change NodeStatus to value-based.

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java

@@ -338,11 +338,9 @@ private NodeStatus newNodeStatus(DatanodeDetails dn,
       LOG.info("Updating nodeOperationalState on registration as the " +
               "datanode has a persisted state of {} and expiry of {}",
           dnOpState, dn.getPersistedOpStateExpiryEpochSec());
-      return new NodeStatus(dnOpState, state,
-          dn.getPersistedOpStateExpiryEpochSec());
+      return NodeStatus.valueOf(dnOpState, state, dn.getPersistedOpStateExpiryEpochSec());
     } else {
-      return new NodeStatus(
-          NodeOperationalState.IN_SERVICE, state);
+      return NodeStatus.valueOf(NodeOperationalState.IN_SERVICE, state);
     }
   }
 

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java

@@ -17,204 +17,203 @@
 
 package org.apache.hadoop.hdds.scm.node;
 
-import com.google.common.collect.ImmutableSet;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.DECOMMISSIONED;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.DECOMMISSIONING;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_MAINTENANCE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.DEAD;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.HEALTHY;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.HEALTHY_READONLY;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.STALE;
+
+import java.util.Collections;
+import java.util.EnumMap;
 import java.util.EnumSet;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState;
 
 /**
- * This class is used to capture the current status of a datanode. This
- * includes its health (healthy, stale or dead) and its operation status (
- * in_service, decommissioned and maintenance mode) along with the expiry time
- * for the operational state (used with maintenance mode).
+ * The status of a datanode including {@link NodeState}, {@link NodeOperationalState}
+ * and the expiry time for the operational state,
+ * where the expiry time is used in maintenance mode.
+ * <p>
+ * This class is value-based.
  */
-public class NodeStatus implements Comparable<NodeStatus> {
-
-  private static final Set<HddsProtos.NodeOperationalState>
-      MAINTENANCE_STATES = ImmutableSet.copyOf(EnumSet.of(
-          HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
-          HddsProtos.NodeOperationalState.IN_MAINTENANCE
-      ));
-
-  private static final Set<HddsProtos.NodeOperationalState>
-      DECOMMISSION_STATES = ImmutableSet.copyOf(EnumSet.of(
-          HddsProtos.NodeOperationalState.DECOMMISSIONING,
-          HddsProtos.NodeOperationalState.DECOMMISSIONED
-      ));
-
-  private static final Set<HddsProtos.NodeOperationalState>
-      OUT_OF_SERVICE_STATES = ImmutableSet.copyOf(EnumSet.of(
-          HddsProtos.NodeOperationalState.DECOMMISSIONING,
-          HddsProtos.NodeOperationalState.DECOMMISSIONED,
-          HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
-          HddsProtos.NodeOperationalState.IN_MAINTENANCE
-      ));
-
-  public static Set<HddsProtos.NodeOperationalState> maintenanceStates() {
-    return MAINTENANCE_STATES;
+public final class NodeStatus implements Comparable<NodeStatus> {
+  /** For the {@link NodeStatus} objects with {@link #opStateExpiryEpochSeconds} == 0. */
+  private static final Map<NodeOperationalState, Map<NodeState, NodeStatus>> CONSTANT_MAP;
+  static {
+    final Map<NodeOperationalState, Map<NodeState, NodeStatus>> map = new EnumMap<>(NodeOperationalState.class);
+    for (NodeOperationalState op : NodeOperationalState.values()) {
+      final EnumMap<NodeState, NodeStatus> healthMap = new EnumMap<>(NodeState.class);
+      for (NodeState health : NodeState.values()) {
+        healthMap.put(health, new NodeStatus(health, op, 0));
+      }
+      map.put(op, healthMap);
+    }
+    CONSTANT_MAP = map;
   }
 
-  public static Set<HddsProtos.NodeOperationalState> decommissionStates() {
-    return DECOMMISSION_STATES;
+  /** @return a {@link NodeStatus} object with {@link #opStateExpiryEpochSeconds} == 0. */
+  public static NodeStatus valueOf(NodeOperationalState op, NodeState health) {
+    return CONSTANT_MAP.get(op).get(health);
   }
 
-  public static Set<HddsProtos.NodeOperationalState> outOfServiceStates() {
-    return OUT_OF_SERVICE_STATES;
+  /** @return a {@link NodeStatus} object. */
+  public static NodeStatus valueOf(NodeOperationalState op, NodeState health, long opExpiryEpochSeconds) {
+    return opExpiryEpochSeconds == 0 ? valueOf(op, health)
+        : new NodeStatus(health, op, opExpiryEpochSeconds);
   }
 
-  private HddsProtos.NodeOperationalState operationalState;
-  private HddsProtos.NodeState health;
-  private long opStateExpiryEpochSeconds;
+  private static final Set<NodeOperationalState> MAINTENANCE_STATES = Collections.unmodifiableSet(
+      EnumSet.of(ENTERING_MAINTENANCE, IN_MAINTENANCE));
+  /**
+   * @return the set consists of {@link NodeOperationalState#ENTERING_MAINTENANCE}
+   *                         and {@link NodeOperationalState#IN_MAINTENANCE}.
+   */
+  public static Set<NodeOperationalState> maintenanceStates() {
+    return MAINTENANCE_STATES;
+  }
 
-  public NodeStatus(HddsProtos.NodeOperationalState operationalState,
-                    HddsProtos.NodeState health) {
-    this.operationalState = operationalState;
-    this.health = health;
-    this.opStateExpiryEpochSeconds = 0;
+  private static final Set<NodeOperationalState> DECOMMISSION_STATES = Collections.unmodifiableSet(
+      EnumSet.of(DECOMMISSIONING, DECOMMISSIONED));
+  /**
+   * @return the set consists of {@link NodeOperationalState#DECOMMISSIONING}
+   *                         and {@link NodeOperationalState#DECOMMISSIONED}.
+   */
+  public static Set<NodeOperationalState> decommissionStates() {
+    return DECOMMISSION_STATES;
   }
 
-  public NodeStatus(HddsProtos.NodeOperationalState operationalState,
-                    HddsProtos.NodeState health,
-                    long opStateExpireEpocSeconds) {
-    this.operationalState = operationalState;
-    this.health = health;
-    this.opStateExpiryEpochSeconds = opStateExpireEpocSeconds;
+  private static final Set<NodeOperationalState> OUT_OF_SERVICE_STATES = Collections.unmodifiableSet(
+      EnumSet.of(DECOMMISSIONING, DECOMMISSIONED, ENTERING_MAINTENANCE, IN_MAINTENANCE));
+  /**
+   * @return the set consists of {@link NodeOperationalState#DECOMMISSIONING},
+   *                             {@link NodeOperationalState#DECOMMISSIONED},
+   *                             {@link NodeOperationalState#ENTERING_MAINTENANCE}
+   *                         and {@link NodeOperationalState#IN_MAINTENANCE}.
+   */
+  public static Set<NodeOperationalState> outOfServiceStates() {
+    return OUT_OF_SERVICE_STATES;
   }
 
+  private static final NodeStatus IN_SERVICE_HEALTHY = valueOf(IN_SERVICE, HEALTHY);
+  /** @return the status of {@link NodeOperationalState#IN_SERVICE} and {@link NodeState#HEALTHY}. */
   public static NodeStatus inServiceHealthy() {
-    return new NodeStatus(HddsProtos.NodeOperationalState.IN_SERVICE,
-        HddsProtos.NodeState.HEALTHY);
+    return IN_SERVICE_HEALTHY;
   }
 
+  private static final NodeStatus IN_SERVICE_HEALTHY_READONLY = valueOf(IN_SERVICE, HEALTHY_READONLY);
+  /** @return the status of {@link NodeOperationalState#IN_SERVICE} and {@link NodeState#HEALTHY_READONLY}. */
   public static NodeStatus inServiceHealthyReadOnly() {
-    return new NodeStatus(HddsProtos.NodeOperationalState.IN_SERVICE,
-        HddsProtos.NodeState.HEALTHY_READONLY);
+    return IN_SERVICE_HEALTHY_READONLY;
   }
 
+  private static final NodeStatus IN_SERVICE_STALE = NodeStatus.valueOf(IN_SERVICE, STALE);
+  /** @return the status of {@link NodeOperationalState#IN_SERVICE} and {@link NodeState#STALE}. */
   public static NodeStatus inServiceStale() {
-    return new NodeStatus(HddsProtos.NodeOperationalState.IN_SERVICE,
-        HddsProtos.NodeState.STALE);
+    return IN_SERVICE_STALE;
   }
 
+  private static final NodeStatus IN_SERVICE_DEAD = NodeStatus.valueOf(IN_SERVICE, DEAD);
+  /** @return the status of {@link NodeOperationalState#IN_SERVICE} and {@link NodeState#DEAD}. */
   public static NodeStatus inServiceDead() {
-    return new NodeStatus(HddsProtos.NodeOperationalState.IN_SERVICE,
-        HddsProtos.NodeState.DEAD);
+    return IN_SERVICE_DEAD;
+  }
+
+  private final NodeState health;
+  private final NodeOperationalState operationalState;
+  private final long opStateExpiryEpochSeconds;
+
+  private NodeStatus(NodeState health, NodeOperationalState op, long opExpiryEpochSeconds) {
+    this.health = health;
+    this.operationalState = op;
+    this.opStateExpiryEpochSeconds = opExpiryEpochSeconds;
   }
 
+  /** Is this node writeable ({@link NodeState#HEALTHY} and {@link NodeOperationalState#IN_SERVICE}) ? */
   public boolean isNodeWritable() {
-    return health == HddsProtos.NodeState.HEALTHY &&
-        operationalState == HddsProtos.NodeOperationalState.IN_SERVICE;
+    return health == HEALTHY && operationalState == IN_SERVICE;
   }
 
-  public HddsProtos.NodeState getHealth() {
+  public NodeState getHealth() {
     return health;
   }
 
-  public HddsProtos.NodeOperationalState getOperationalState() {
+  public NodeOperationalState getOperationalState() {
     return operationalState;
   }
 
   public long getOpStateExpiryEpochSeconds() {
     return opStateExpiryEpochSeconds;
   }
 
+  /** Is the op expired? */
   public boolean operationalStateExpired() {
-    if (0 == opStateExpiryEpochSeconds) {
-      return false;
-    }
-    return System.currentTimeMillis() / 1000 >= opStateExpiryEpochSeconds;
+    return opStateExpiryEpochSeconds != 0
+        && System.currentTimeMillis() >= 1000 * opStateExpiryEpochSeconds;
   }
 
+  /** @return true iff the node is {@link NodeOperationalState#IN_SERVICE}. */
   public boolean isInService() {
-    return operationalState == HddsProtos.NodeOperationalState.IN_SERVICE;
+    return operationalState == IN_SERVICE;
   }
 
   /**
-   * Returns true if the nodeStatus indicates the node is in any decommission
-   * state.
-   *
-   * @return True if the node is in any decommission state, false otherwise
+   * @return true iff the node is {@link NodeOperationalState#DECOMMISSIONING}
+   *                           or {@link NodeOperationalState#DECOMMISSIONED}.
    */
   public boolean isDecommission() {
     return DECOMMISSION_STATES.contains(operationalState);
   }
 
-  /**
-   * Returns true if the node is currently decommissioning.
-   *
-   * @return True if the node is decommissioning, false otherwise
-   */
+  /** @return true iff the node is {@link NodeOperationalState#DECOMMISSIONING}. */
   public boolean isDecommissioning() {
-    return operationalState == HddsProtos.NodeOperationalState.DECOMMISSIONING;
+    return operationalState == DECOMMISSIONING;
   }
 
-  /**
-   * Returns true if the node is decommissioned.
-   *
-   * @return True if the node is decommissioned, false otherwise
-   */
+  /** @return true iff the node is {@link NodeOperationalState#DECOMMISSIONED}. */
   public boolean isDecommissioned() {
-    return operationalState == HddsProtos.NodeOperationalState.DECOMMISSIONED;
+    return operationalState == DECOMMISSIONED;
   }
 
   /**
-   * Returns true if the node is in any maintenance state.
-   *
-   * @return True if the node is in any maintenance state, false otherwise
+   * @return true iff the node is {@link NodeOperationalState#ENTERING_MAINTENANCE}
+   *                           or {@link NodeOperationalState#IN_MAINTENANCE}.
    */
   public boolean isMaintenance() {
-    return MAINTENANCE_STATES.contains(operationalState);
+    return maintenanceStates().contains(operationalState);
   }
 
-  /**
-   * Returns true if the node is currently entering maintenance.
-   *
-   * @return True if the node is entering maintenance, false otherwise
-   */
+  /** @return true iff the node is {@link NodeOperationalState#ENTERING_MAINTENANCE}. */
   public boolean isEnteringMaintenance() {
-    return operationalState
-        == HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE;
+    return operationalState == ENTERING_MAINTENANCE;
   }
 
-  /**
-   * Returns true if the node is currently in maintenance.
-   *
-   * @return True if the node is in maintenance, false otherwise.
-   */
+  /** @return true iff the node is {@link NodeOperationalState#IN_MAINTENANCE}. */
   public boolean isInMaintenance() {
-    return operationalState == HddsProtos.NodeOperationalState.IN_MAINTENANCE;
+    return operationalState == IN_MAINTENANCE;
   }
 
-  /**
-   * Returns true if the nodeStatus is healthy or healthy_readonly (ie not stale
-   * or dead) and false otherwise.
-   *
-   * @return True if the node is healthy or healthy_readonly, false otherwise.
-   */
+  /** @return true iff this node is {@link NodeState#HEALTHY} or {@link NodeState#HEALTHY_READONLY}. */
   public boolean isHealthy() {
-    return health == HddsProtos.NodeState.HEALTHY
-        || health == HddsProtos.NodeState.HEALTHY_READONLY;
+    return health == HEALTHY
+        || health == HEALTHY_READONLY;
   }
 
-  /**
-   * Returns true if the nodeStatus is either healthy or stale and false
-   * otherwise.
-   *
-   * @return True is the node is Healthy or Stale, false otherwise.
-   */
+  /** @return true iff this node is {@link NodeState#HEALTHY} or {@link NodeState#STALE}. */
   public boolean isAlive() {
-    return health == HddsProtos.NodeState.HEALTHY
-        || health == HddsProtos.NodeState.STALE;
+    return health == HEALTHY
+        || health == STALE;
   }
 
-  /**
-   * Returns true if the nodeStatus is dead and false otherwise.
-   *
-   * @return True is the node is Dead, false otherwise.
-   */
+  /** @return true iff the node is {@link NodeState#DEAD}. */
   public boolean isDead() {
-    return health == HddsProtos.NodeState.DEAD;
+    return health == DEAD;
   }
 
   @Override
@@ -228,13 +227,10 @@ public boolean equals(Object obj) {
     if (getClass() != obj.getClass()) {
       return false;
     }
-    NodeStatus other = (NodeStatus) obj;
-    if (this.operationalState == other.operationalState &&
-        this.health == other.health
-        && this.opStateExpiryEpochSeconds == other.opStateExpiryEpochSeconds) {
-      return true;
-    }
-    return false;
+    final NodeStatus that = (NodeStatus) obj;
+    return this.operationalState == that.operationalState
+        && this.health == that.health
+        && this.opStateExpiryEpochSeconds == that.opStateExpiryEpochSeconds;
   }
 
   @Override
@@ -244,8 +240,8 @@ public int hashCode() {
 
   @Override
   public String toString() {
-    return "OperationalState: " + operationalState + " Health: " + health +
-        " OperationStateExpiry: " + opStateExpiryEpochSeconds;
+    return "OperationalState: " + operationalState
+        + "(expiry: " + opStateExpiryEpochSeconds + "s), Health: " + health;
   }
 
   @Override

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java

@@ -146,8 +146,7 @@ public NodeStatus updateNodeHealthState(UUID nodeId, NodeState newHealth)
       lock.writeLock().lock();
       DatanodeInfo dn = getNodeInfoUnsafe(nodeId);
       NodeStatus oldStatus = dn.getNodeStatus();
-      NodeStatus newStatus = new NodeStatus(
-          oldStatus.getOperationalState(), newHealth);
+      NodeStatus newStatus = NodeStatus.valueOf(oldStatus.getOperationalState(), newHealth);
       dn.setNodeStatus(newStatus);
       return newStatus;
     } finally {
@@ -170,8 +169,7 @@ public NodeStatus updateNodeOperationalState(UUID nodeId,
       lock.writeLock().lock();
       DatanodeInfo dn = getNodeInfoUnsafe(nodeId);
       NodeStatus oldStatus = dn.getNodeStatus();
-      NodeStatus newStatus = new NodeStatus(
-          newOpState, oldStatus.getHealth(), opStateExpiryEpochSeconds);
+      NodeStatus newStatus = NodeStatus.valueOf(newOpState, oldStatus.getHealth(), opStateExpiryEpochSeconds);
       dn.setNodeStatus(newStatus);
       return newStatus;
     } finally {
@@ -440,7 +438,7 @@ private void checkIfNodeExist(UUID uuid) throws NodeNotFoundException {
   private List<DatanodeInfo> filterNodes(
       NodeOperationalState opState, NodeState health) {
     if (opState != null && health != null) {
-      return filterNodes(matching(new NodeStatus(opState, health)));
+      return filterNodes(matching(NodeStatus.valueOf(opState, health)));
     }
     if (opState != null) {
       return filterNodes(matching(opState));