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/03/16 11:57:38 UTC

[GitHub] [ozone] cchenax opened a new pull request #2051: Display key offset for each block in command key info

cchenax opened a new pull request #2051:
URL: https://github.com/apache/ozone/pull/2051


   ## What changes were proposed in this pull request?
   
   add keyOffset in the key info result
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4983
   
   ## How was this patch tested?
   example:sh key info /vol1/bucket1/key1
   
   {
     "volumeName" : "vol1",
     "bucketName" : "bucket1",
     "name" : "key1",
     "dataSize" : 16,
     "creationTime" : "2021-03-12T11:23:22.125Z",
     "modificationTime" : "2021-03-16T08:28:08.901Z",
     "replicationType" : "RATIS",
     "replicationFactor" : 3,
     "ozoneKeyLocations" : [ {
       "containerID" : 5,
       "localID" : 105898527115771904,
       "length" : 16,
       "offset" : 0,
       "keyOffset" : 16
     } ],
     "metadata" : { },
     "fileEncryptionInfo" : null
   }
   


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

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 #2051: HDDS-4983. Display key offset for each block in command key info

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


   > For the records: jira issue has a conversation about improving the initial implementation:
   > 
   > ![image](https://user-images.githubusercontent.com/170549/114026897-16718280-9877-11eb-81c2-0f38f2e51c71.png)
   
   yes,I will improve it.


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

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] elek merged pull request #2051: HDDS-4983. Display key offset for each block in command key info

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


   


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

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] elek commented on pull request #2051: HDDS-4983. Display key offset for each block in command key info

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


   For the records: jira issue has a conversation about improving the initial implementation:
   
   ![image](https://user-images.githubusercontent.com/170549/114026897-16718280-9877-11eb-81c2-0f38f2e51c71.png)
   


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

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] elek commented on a change in pull request #2051: HDDS-4983. Display key offset for each block in command key info

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -858,9 +859,17 @@ public OzoneKeyDetails getKeyDetails(
     OmKeyInfo keyInfo = ozoneManagerClient.lookupKey(keyArgs);
 
     List<OzoneKeyLocation> ozoneKeyLocations = new ArrayList<>();
-    keyInfo.getLatestVersionLocations().getBlocksLatestVersionOnly().forEach(
-        (a) -> ozoneKeyLocations.add(new OzoneKeyLocation(a.getContainerID(),
-            a.getLocalID(), a.getLength(), a.getOffset())));
+    long lastKeyOffset = 0L;
+    long lastLength = 0L;
+    List<OmKeyLocationInfo> omKeyLocationInfos = keyInfo
+        .getLatestVersionLocations().getBlocksLatestVersionOnly();
+    for (OmKeyLocationInfo info: omKeyLocationInfos) {
+      ozoneKeyLocations.add(new OzoneKeyLocation(info.getContainerID(),
+          info.getLocalID(), info.getLength(), info.getOffset(),
+          lastKeyOffset + lastLength));
+      lastKeyOffset = lastLength + lastKeyOffset;
+      lastLength = info.getLength();
+    }

Review comment:
       Fix me if I am wrong, but based on my understanding the variable usage can be simplified in this way:
   
   ```suggestion
       long lastKeyOffset = 0L;
       List<OmKeyLocationInfo> omKeyLocationInfos = keyInfo
           .getLatestVersionLocations().getBlocksLatestVersionOnly();
       for (OmKeyLocationInfo info: omKeyLocationInfos) {
         ozoneKeyLocations.add(new OzoneKeyLocation(info.getContainerID(),
             info.getLocalID(), info.getLength(), info.getOffset(),
             lastKeyOffset));
         lastKeyOffset += info.getLength();
       }
   ```




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

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] cku328 commented on a change in pull request #2051: HDDS-4983. Display key offset for each block in command key info

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneKeyLocation.java
##########
@@ -39,16 +39,20 @@
    * Offset of this key.
    */
   private final long offset;
-
+  /**
+   * KeyOffset of this key.
+   */
+  private final long KeyOffset;

Review comment:
       ```suggestion
     private final long keyOffset;
   ```
   use lower camel case: KeyOffset -> keyOffset

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneKeyLocation.java
##########
@@ -39,16 +39,20 @@
    * Offset of this key.
    */
   private final long offset;
-
+  /**
+   * KeyOffset of this key.
+   */
+  private final long KeyOffset;
   /**
    * Constructs OzoneKeyLocation.
    */
   public OzoneKeyLocation(long containerID, long localID,
-                  long length, long offset) {
+                          long length, long offset) {

Review comment:
       Nit: seems like an unnecessary adjustment.

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneKeyLocation.java
##########
@@ -79,4 +83,9 @@ public long getOffset() {
     return offset;
   }
 
+  /**
+   * Returns the KeyOffset of this Key.
+   */
+  public long getKeyOffset() { return KeyOffset; }

Review comment:
       ```suggestion
     public long getKeyOffset() { 
       return keyOffset;
     }
   ```
   blocks should have line breaks.




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

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 #2051: HDDS-4983. Display key offset for each block in command key info

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


   > Thanks the patch @cchenax
   > 
   > Overall it looks good to me, I checked the class and it's used only on the client side, safe to modify.
   > 
   > I have one question about simplifying the loop.
   
   ok,this way is good,thank you very much.


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

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