You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/09/02 02:40:49 UTC

[GitHub] [ozone] ChenSammi opened a new pull request #2606: HDDS-5707. List container supports replication factor filter.

ChenSammi opened a new pull request #2606:
URL: https://github.com/apache/ozone/pull/2606


   https://issues.apache.org/jira/browse/HDDS-5707


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


[GitHub] [ozone] captainzmc commented on pull request #2606: HDDS-5707. List container supports replication factor filter.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2606:
URL: https://github.com/apache/ozone/pull/2606#issuecomment-952581952


   Merged this, thanks @ChenSammi ‘ patch, and thanks @JacksonYao287  for the review.


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


[GitHub] [ozone] captainzmc merged pull request #2606: HDDS-5707. List container supports replication factor filter.

Posted by GitBox <gi...@apache.org>.
captainzmc merged pull request #2606:
URL: https://github.com/apache/ozone/pull/2606


   


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


[GitHub] [ozone] JacksonYao287 commented on a change in pull request #2606: HDDS-5707. List container supports replication factor filter.

Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2606:
URL: https://github.com/apache/ozone/pull/2606#discussion_r701825274



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -379,21 +379,53 @@ private boolean hasRequiredReplicas(ContainerInfo contInfo) {
   @Override
   public List<ContainerInfo> listContainer(long startContainerID,
       int count, HddsProtos.LifeCycleState state) throws IOException {
+    return listContainer(startContainerID, count, state, null);
+  }
+
+  /**
+   * Lists a range of containers and get their info.
+   *
+   * @param startContainerID start containerID.
+   * @param count count must be {@literal >} 0.
+   * @param state Container with this state will be returned.
+   * @param factor Container factor.
+   * @return a list of pipeline.
+   * @throws IOException
+   */
+  @Override
+  public List<ContainerInfo> listContainer(long startContainerID,
+      int count, HddsProtos.LifeCycleState state,
+      HddsProtos.ReplicationFactor factor) throws IOException {
     boolean auditSuccess = true;
     Map<String, String> auditMap = Maps.newHashMap();
     auditMap.put("startContainerID", String.valueOf(startContainerID));
     auditMap.put("count", String.valueOf(count));
     if (state != null) {
       auditMap.put("state", state.name());
     }
+    if (factor != null) {
+      auditMap.put("factor", factor.name());
+    }
     try {
       final ContainerID containerId = ContainerID.valueOf(startContainerID);
-      if(null == state) {
+      if(state != null && factor != null) {
+        return scm.getContainerManager().getContainers(state).stream()
+            .filter(info -> info.containerID().getId() >= startContainerID)
+            .filter(info -> (info.getReplicationFactor() == factor))
+            .sorted().limit(count).collect(Collectors.toList());
+      } else if (state == null && factor == null) {
         return scm.getContainerManager().getContainers(containerId, count);
+      } else if (state != null) {
+        return scm.getContainerManager().getContainers(state).stream()
+            .filter(info -> info.containerID().getId() >= startContainerID)
+            .sorted().limit(count).collect(Collectors.toList());
+      } else {
+        // factor != null
+        return scm.getContainerManager().getContainers().stream()
+            .filter(info -> info.containerID().getId() >= startContainerID)
+            .filter(info -> info.getReplicationFactor() == factor)
+            .sorted().limit(count).collect(Collectors.toList());

Review comment:
       maybe we can refactor the codes as below, i think it will make the logic more clear and efficient!
   ```
   if (state == null){
       if(factor == null){
           .............
       } else{
            ............
       }
   }else{
       if(factor == null){
          ...........
       } else{
             .......
       }
   }
   ```




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


[GitHub] [ozone] ChenSammi commented on a change in pull request #2606: HDDS-5707. List container supports replication factor filter.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2606:
URL: https://github.com/apache/ozone/pull/2606#discussion_r707901111



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -379,21 +379,53 @@ private boolean hasRequiredReplicas(ContainerInfo contInfo) {
   @Override
   public List<ContainerInfo> listContainer(long startContainerID,
       int count, HddsProtos.LifeCycleState state) throws IOException {
+    return listContainer(startContainerID, count, state, null);
+  }
+
+  /**
+   * Lists a range of containers and get their info.
+   *
+   * @param startContainerID start containerID.
+   * @param count count must be {@literal >} 0.
+   * @param state Container with this state will be returned.
+   * @param factor Container factor.
+   * @return a list of pipeline.
+   * @throws IOException
+   */
+  @Override
+  public List<ContainerInfo> listContainer(long startContainerID,
+      int count, HddsProtos.LifeCycleState state,
+      HddsProtos.ReplicationFactor factor) throws IOException {
     boolean auditSuccess = true;
     Map<String, String> auditMap = Maps.newHashMap();
     auditMap.put("startContainerID", String.valueOf(startContainerID));
     auditMap.put("count", String.valueOf(count));
     if (state != null) {
       auditMap.put("state", state.name());
     }
+    if (factor != null) {
+      auditMap.put("factor", factor.name());
+    }
     try {
       final ContainerID containerId = ContainerID.valueOf(startContainerID);
-      if(null == state) {
+      if(state != null && factor != null) {
+        return scm.getContainerManager().getContainers(state).stream()
+            .filter(info -> info.containerID().getId() >= startContainerID)
+            .filter(info -> (info.getReplicationFactor() == factor))
+            .sorted().limit(count).collect(Collectors.toList());
+      } else if (state == null && factor == null) {
         return scm.getContainerManager().getContainers(containerId, count);
+      } else if (state != null) {
+        return scm.getContainerManager().getContainers(state).stream()
+            .filter(info -> info.containerID().getId() >= startContainerID)
+            .sorted().limit(count).collect(Collectors.toList());
+      } else {
+        // factor != null
+        return scm.getContainerManager().getContainers().stream()
+            .filter(info -> info.containerID().getId() >= startContainerID)
+            .filter(info -> info.getReplicationFactor() == factor)
+            .sorted().limit(count).collect(Collectors.toList());

Review comment:
       Thanks @JacksonYao287 for the code review. I will update a new commit later to addres the comments. 




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