You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sarvekshayr (via GitHub)" <gi...@apache.org> on 2026/07/13 05:22:56 UTC

[PR] HDDS-15837. Add health-aware container ID listing in SCM [ozone]

sarvekshayr opened a new pull request, #10736:
URL: https://github.com/apache/ozone/pull/10736

   ## What changes were proposed in this pull request?
   Adds optional health-state filtering to SCM’s in-process container ID listing APIs. 
   
   ContainerStateMap gains a 4-argument `getContainerIDs(lifecycle, health, start, count)` with three dispatch paths:
   
   - Lifecycle only (health == null) - uses the existing lifecycle index (same behavior as before).
   - Lifecycle + health - walks the lifecycle index and filters by `ContainerInfo.getHealthState()`.
   - Health only (lifecycle == null) - scans the full container map (no health index; documented as slower at scale).
   
   The new filter is threaded through `ContainerManager`, `ContainerStateManager`, and the HA `ContainerStateManagerInvoker`. The existing `listContainerIDs` RPC path passes null for health, so external behavior is unchanged.
   
   Part **2 of 5** in splitting [HDDS-15496](https://issues.apache.org/jira/browse/HDDS-15496) https://github.com/apache/ozone/pull/10673 container ID export. 
   
   ## What is the link to the Apache JIRA
   [HDDS-15837](https://issues.apache.org/jira/browse/HDDS-15837)
   
   ## How was this patch tested?
   Added unit tests.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-15837. Add health-aware container ID listing in SCM [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #10736:
URL: https://github.com/apache/ozone/pull/10736#discussion_r3576861219


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/states/ContainerStateMap.java:
##########
@@ -118,6 +120,30 @@ List<ContainerInfo> getInfos(ContainerID start, int count) {
           .collect(Collectors.toList());
     }
 
+    List<ContainerID> getFilteredContainerIDs(ContainerID start, int count,
+        LifeCycleState lifeCycleState, ContainerHealthState healthState) {

Review Comment:
   - lifeCycleState always null.
   - Remove "Filtered" from the name
   ```java
       List<ContainerID> getContainerIDs(ContainerID start, int count, ContainerHealthState healthState) {
         Objects.requireNonNull(start, "start == null");
         Preconditions.assertTrue(count >= 0, "count < 0");
   
         final List<ContainerID> result = new ArrayList<>(1024);
         for (ContainerEntry entry : map.tailMap(start).values()) {
           ContainerInfo info = entry.getInfo();
           if (healthState == null || info.getHealthState() != healthState) {
             result.add(info.containerID());
             if (result.size() >= count) {
               break;
             }
           }
         }
         return result;
       }
   ```



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/states/ContainerStateMap.java:
##########
@@ -118,6 +120,30 @@ List<ContainerInfo> getInfos(ContainerID start, int count) {
           .collect(Collectors.toList());
     }
 
+    List<ContainerID> getFilteredContainerIDs(ContainerID start, int count,
+        LifeCycleState lifeCycleState, ContainerHealthState healthState) {
+      Objects.requireNonNull(start, "start == null");
+      Preconditions.assertTrue(count >= 0, "count < 0");
+      List<ContainerID> result = new ArrayList<>(Math.min(count, 64));

Review Comment:
   Use 1024



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManager.java:
##########
@@ -106,11 +106,14 @@ public interface ContainerStateManager extends SCMHandler {
   /**
    * Get {@link ContainerID}s for the given state.
    *
+   * @param state optional lifecycle filter
+   * @param healthState optional health filter

Review Comment:
   Use LifeCycleState and ContainerHealthState.  The word "filter" is quite confusing since all the other methods do not have it.  Please do not use it.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/states/ContainerStateMap.java:
##########
@@ -118,6 +120,30 @@ List<ContainerInfo> getInfos(ContainerID start, int count) {
           .collect(Collectors.toList());
     }
 
+    List<ContainerID> getFilteredContainerIDs(ContainerID start, int count,
+        LifeCycleState lifeCycleState, ContainerHealthState healthState) {
+      Objects.requireNonNull(start, "start == null");
+      Preconditions.assertTrue(count >= 0, "count < 0");
+      List<ContainerID> result = new ArrayList<>(Math.min(count, 64));
+      if (count == 0) {
+        return result;
+      }

Review Comment:
   count == 0 should be checked in the public method.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/states/ContainerStateMap.java:
##########
@@ -274,6 +300,44 @@ public List<ContainerID> getContainerIDs(LifeCycleState state, ContainerID start
         .collect(Collectors.toList());
   }
 
+  private List<ContainerID> getContainerIDsFromLifecycleIndex(LifeCycleState lifeCycleState,
+      ContainerHealthState healthState, ContainerID start, int count) {
+    List<ContainerID> result = new ArrayList<>(Math.min(count, 64));
+    if (count == 0) {
+      return result;
+    }
+    for (ContainerInfo info : lifeCycleStateMap.tailMap(lifeCycleState, start).values()) {
+      if (info.getHealthState() == healthState) {
+        result.add(info.containerID());
+        if (result.size() >= count) {
+          break;
+        }
+      }
+    }
+    return result;
+  }
+
+  /**
+   * Returns container IDs matching optional lifecycle and health filters,
+   * in ascending {@link ContainerID} order starting from {@code start}
+   * (inclusive).
+   * Lifecycle-only and lifecycle + health filters use the lifecycle index.
+   * Health-only filters scan the full container map (no health index); prefer
+   * supplying a lifecycle filter at scale.
+   */
+  public List<ContainerID> getContainerIDs(LifeCycleState lifeCycleState,
+      ContainerHealthState healthState, ContainerID start, int count) {
+    Preconditions.assertTrue(count >= 0, "count < 0");
+    if (healthState == null && lifeCycleState != null) {
+      return getContainerIDs(lifeCycleState, start, count);
+    }
+    if (healthState != null && lifeCycleState != null) {
+      return getContainerIDsFromLifecycleIndex(lifeCycleState, healthState, start, count);
+    }
+    return containerMap.getFilteredContainerIDs(start, count, lifeCycleState,
+        healthState);
+  }

Review Comment:
   Remove getContainerIDsFromLifecycleIndex and simplify this method as as below:
   ```java
     /**
      * Returns container IDs matching given optional lifeCycleState and healthState,
      * in ascending {@link ContainerID} order starting from {@code start} (inclusive).
      *
      * @param start the start id
      * @param count the maximum size of the returned list
      * @return a list of sorted {@link ContainerID}s
      */
     public List<ContainerID> getContainerIDs(LifeCycleState lifeCycleState,
         ContainerHealthState healthState, ContainerID start, int count) {
       if (count == 0) {
         return Collections.emptyList();
       }
       Preconditions.assertTrue(count > 0, "count < 0");
   
       if (lifeCycleState == null) {
         return containerMap.getContainerIDs(start, count, healthState);
       }
   
       final List<ContainerID> result = new ArrayList<>(Math.min(count, 1024));
       for (ContainerInfo info : lifeCycleStateMap.tailMap(lifeCycleState, start).values()) {
         if (healthState == null || info.getHealthState() == healthState) {
           result.add(info.containerID());
           if (result.size() >= count) {
             break;
           }
         }
       }
       return result;
     }
   ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-15837. Add health-aware container ID listing in SCM [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #10736:
URL: https://github.com/apache/ozone/pull/10736


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org