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/11/11 02:09:44 UTC

[GitHub] [ozone] ChenSammi opened a new pull request #2829: HDDS-5963. Implement ListBlock command.

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


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


-- 
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] adoroszlai commented on pull request #2829: HDDS-5963. Implement ListBlock command.

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


   /pending


-- 
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] cchenax edited a comment on pull request #2829: HDDS-5963. Implement ListBlock command.

Posted by GitBox <gi...@apache.org>.
cchenax edited a comment on pull request #2829:
URL: https://github.com/apache/ozone/pull/2829#issuecomment-1072004726


   because of I can not commit code the sammi's pr, So I take this pr,I take the new one.@[adoroszlai](https://github.com/adoroszlai)


-- 
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] cchenax commented on pull request #2829: HDDS-5963. Implement ListBlock command.

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


   because of I can not commit code the sammi's pr, So I take this pr,I take the new one.


-- 
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] cchenax commented on pull request #2829: HDDS-5963. Implement ListBlock command.

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


   because of I can not commit code the sammi's pr, So I take this pr,I take the new one.


-- 
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] adoroszlai commented on a change in pull request #2829: HDDS-5963. Implement ListBlock command.

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



##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java
##########
@@ -380,4 +399,31 @@ private ContainerCommandRequestProto getReadChunkRequest(
         .build();
   }
 
+  private ContainerCommandRequestProto getPutBlockRequest(
+      ContainerCommandRequestProto writeChunkRequest) {
+    ContainerProtos.BlockData.Builder block =
+        ContainerProtos.BlockData.newBuilder()
+            .setSize(writeChunkRequest.getWriteChunk().getChunkData().getLen())
+            .setBlockID(writeChunkRequest.getWriteChunk().getBlockID())
+            .addChunks(writeChunkRequest.getWriteChunk().getChunkData());
+    return ContainerCommandRequestProto.newBuilder()
+        .setContainerID(writeChunkRequest.getContainerID())
+        .setCmdType(ContainerProtos.Type.PutBlock)
+        .setDatanodeUuid(writeChunkRequest.getDatanodeUuid())
+        .setPutBlock(ContainerProtos.PutBlockRequestProto.newBuilder()
+            .setBlockData(block.build())
+            .build())
+        .build();
+  }
+
+  private ContainerCommandRequestProto getListBlockRequest(
+      ContainerCommandRequestProto writeChunkRequest) {
+    return ContainerCommandRequestProto.newBuilder()
+        .setContainerID(writeChunkRequest.getContainerID())
+        .setCmdType(ContainerProtos.Type.ListBlock)
+        .setDatanodeUuid(writeChunkRequest.getDatanodeUuid())
+        .setListBlock(ContainerProtos.ListBlockRequestProto.newBuilder()
+            .setCount(10).build())
+        .build();
+  }

Review comment:
       Can you please move this to `ContainerTestHelper`?

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java
##########
@@ -165,6 +166,18 @@ public static ContainerCommandResponseProto getBlockDataResponse(
         .build();
   }
 
+  public static ContainerCommandResponseProto getListBlockResponse(
+      ContainerCommandRequestProto msg, List<BlockData> data) {
+
+    ListBlockResponseProto.Builder builder =
+        ListBlockResponseProto.newBuilder();
+    for(int i = 0; i < data.size(); i++) {
+      builder.addBlockData(data.get(i));
+    }

Review comment:
       Nit: we can add all items at once.
   
   ```suggestion
       builder.addAllBlockData(data);
   ```

##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java
##########
@@ -380,4 +399,31 @@ private ContainerCommandRequestProto getReadChunkRequest(
         .build();
   }
 
+  private ContainerCommandRequestProto getPutBlockRequest(
+      ContainerCommandRequestProto writeChunkRequest) {
+    ContainerProtos.BlockData.Builder block =
+        ContainerProtos.BlockData.newBuilder()
+            .setSize(writeChunkRequest.getWriteChunk().getChunkData().getLen())
+            .setBlockID(writeChunkRequest.getWriteChunk().getBlockID())
+            .addChunks(writeChunkRequest.getWriteChunk().getChunkData());
+    return ContainerCommandRequestProto.newBuilder()
+        .setContainerID(writeChunkRequest.getContainerID())
+        .setCmdType(ContainerProtos.Type.PutBlock)
+        .setDatanodeUuid(writeChunkRequest.getDatanodeUuid())
+        .setPutBlock(ContainerProtos.PutBlockRequestProto.newBuilder()
+            .setBlockData(block.build())
+            .build())
+        .build();
+  }

Review comment:
       Please reuse existing code from `ContainerTestHelper`:
   
   https://github.com/apache/ozone/blob/e6179c1ec6d77a46687c513ab63b81d7c62e9796/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java#L408-L462

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -545,6 +546,43 @@ ContainerCommandResponseProto handleGetCommittedBlockLength(
     return getBlockLengthResponse(request, blockLength);
   }
 
+  /**
+   * Handle List Block operation. Calls BlockManager to process the request.
+   */
+  ContainerCommandResponseProto handleListBlock(
+      ContainerCommandRequestProto request, KeyValueContainer kvContainer) {
+
+    if (!request.hasListBlock()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Malformed list block request. trace ID: {}",
+            request.getTraceID());
+      }
+      return malformedRequest(request);
+    }
+
+    List<BlockData> responseData;
+    List<ContainerProtos.BlockData> returnData = new ArrayList();

Review comment:
       ```suggestion
       List<ContainerProtos.BlockData> returnData = new ArrayList<>();
   ```




-- 
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] cchenax edited a comment on pull request #2829: HDDS-5963. Implement ListBlock command.

Posted by GitBox <gi...@apache.org>.
cchenax edited a comment on pull request #2829:
URL: https://github.com/apache/ozone/pull/2829#issuecomment-1072004726


   because of I can not commit code the sammi's pr, So I take this pr,I take the new one.@[adoroszlai](https://github.com/adoroszlai)
   this pr https://github.com/apache/ozone/pull/3212


-- 
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] adoroszlai closed pull request #2829: HDDS-5963. Implement ListBlock command.

Posted by GitBox <gi...@apache.org>.
adoroszlai closed pull request #2829:
URL: https://github.com/apache/ozone/pull/2829


   


-- 
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] cchenax edited a comment on pull request #2829: HDDS-5963. Implement ListBlock command.

Posted by GitBox <gi...@apache.org>.
cchenax edited a comment on pull request #2829:
URL: https://github.com/apache/ozone/pull/2829#issuecomment-1072004726






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