You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "szetszwo (via GitHub)" <gi...@apache.org> on 2025/03/12 06:15:26 UTC

[PR] HDDS-12555. Combine containerMap and replicaMap in ContainerStateMap. [ozone]

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

   ## What changes were proposed in this pull request?
   
   ContainerStateMap has two maps, containerMap and replicaMap.  Both of them has the same key type ContainerID.  They should be combined into one map for simplicity and efficiency.
   
   ## What is the link to the Apache JIRA
   
   HDDS-12555.
   
   ## How was this patch tested?
   
   Updating existing 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-12555. Combine containerMap and replicaMap in ContainerStateMap. [ozone]

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


-- 
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-12555. Combine containerMap and replicaMap in ContainerStateMap. [ozone]

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


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java:
##########
@@ -413,8 +429,10 @@ public void updateContainerReplica(final ContainerID id,
   @Override
   public void removeContainerReplica(final ContainerID id,
                                      final ContainerReplica replica) {
+    //TODO remove ContainerID parameter

Review Comment:
   Filed HDDS-12572



-- 
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-12555. Combine containerMap and replicaMap in ContainerStateMap. [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #8057:
URL: https://github.com/apache/ozone/pull/8057#issuecomment-2721948186

   @adoroszlai , thanks a lot for reviewing this.  Committed all you suggestions.


-- 
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-12555. Combine containerMap and replicaMap in ContainerStateMap. [ozone]

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


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/states/ContainerStateMap.java:
##########
@@ -214,30 +278,39 @@ public void updateState(ContainerID containerID, LifeCycleState currentState,
     currentInfo.setState(newState);
   }
 
-  public Set<ContainerID> getAllContainerIDs() {
-    return ImmutableSet.copyOf(containerMap.keySet());
+  public List<ContainerInfo> getContainerInfos(ContainerID start, int count) {
+    return containerMap.getInfos(start, count);
   }
 
   /**
-   * Returns Containers in the System by the Type.
    *
-   * @param type - Replication type -- StandAlone, Ratis etc.
-   * @return NavigableSet
+   * @param state the state of the {@link ContainerInfo}s
+   * @param start the start id
+   * @param count the maximum size of the returned list
+   * @return a list of {@link ContainerInfo}s sorted by {@link ContainerID}
    */
-  public NavigableSet<ContainerID> getContainerIDsByType(final ReplicationType type) {
-    Preconditions.checkNotNull(type);
-    return typeMap.getCollection(type);
+  public List<ContainerInfo> getContainerInfos(LifeCycleState state, ContainerID start, int count) {
+    Preconditions.assertTrue(count >= 0, "count < 0");
+    return lifeCycleStateMap.tailSet(state, start).stream()
+        .map(this::getContainerInfo)
+        .limit(count)
+        .collect(Collectors.toList());
   }
 
-  /**
-   * Returns Containers by State.
-   *
-   * @param state - State - Open, Closed etc.
-   * @return List of containers by state.
-   */
-  public NavigableSet<ContainerID> getContainerIDsByState(
-      final LifeCycleState state) {
-    Preconditions.checkNotNull(state);
-    return lifeCycleStateMap.getCollection(state);
+  public List<ContainerInfo> getContainerInfos(LifeCycleState state) {
+    return lifeCycleStateMap.getCollection(state).stream()
+        .map(this::getContainerInfo)
+        .collect(Collectors.toList());
+  }
+
+  public List<ContainerInfo> getContainerInfos(ReplicationType state) {
+    return typeMap.getCollection(state).stream()

Review Comment:
   nit:
   
   ```suggestion
     public List<ContainerInfo> getContainerInfos(ReplicationType type) {
       return typeMap.getCollection(type).stream()
   ```



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManager.java:
##########
@@ -103,22 +104,33 @@ public interface ContainerStateManager {
   boolean contains(ContainerID containerID);
 
   /**
-   * Returns the ID of all the managed containers.
+   * Get {@link ContainerInfo}s.
    *
-   * @return Set of {@link ContainerID}
+   * @param start the start {@link ContainerID} (inclusive)
+   * @param count the size limit
+   * @return a list of {@link ContainerInfo};
    */
-  Set<ContainerID> getContainerIDs();
+  List<ContainerInfo> getContainerInfos(ContainerID start, int count);
 
   /**
+   * Get {@link ContainerInfo}s for the given state.
    *
+   * @param start the start {@link ContainerID} (inclusive)
+   * @param count the size limit
+   * @return a list of {@link ContainerInfo};
    */
-  Set<ContainerID> getContainerIDs(LifeCycleState state);
+  List<ContainerInfo> getContainerInfos(LifeCycleState state, ContainerID start, int count);
 
+  /** @return all {@link ContainerInfo}s for the given state. */
+  List<ContainerInfo> getContainerInfos(LifeCycleState state);
 
   /**
-   * Returns the IDs of the Containers whose ReplicationType matches the given type.
+   * @return all {@link ContainerID}s for the given state.

Review Comment:
   nit: 
   
   ```suggestion
      * @return number of containers for the given state.
   ```



-- 
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