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/10/19 07:39:01 UTC

[GitHub] [ozone] sadanand48 opened a new pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

sadanand48 opened a new pull request #2747:
URL: https://github.com/apache/ozone/pull/2747


   ## What changes were proposed in this pull request?
   When moving a file/dir to trash, the destination path is different for ofs and o3fs.  The change here is to make it same for both.
   o3fs -> /<vol>/<buck>/.Trash/<user>/Current/..<dir if any>..
   ofs -> /<vol>/<buck>/.Trash/<user>/Current/<vol>/<buck>/..<dir if any>..
   
   Omitting volume and bucket for ofs key
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-5866
   
   ## How was this patch tested?
   unit test
   


-- 
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] mukul1987 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       I guess the problem is in
   
     public Path getTrashRoot() {
       if (!this.isKey()) {
         throw new RuntimeException("Volume or bucket doesn't have trash root.");
       }
       try {
         final String username =
                 UserGroupInformation.getCurrentUser().getShortUserName();
         final Path pathRoot = new Path(
             OZONE_OFS_URI_SCHEME, authority, OZONE_URI_DELIMITER);
         final Path pathToVolume = new Path(pathRoot, volumeName);
         final Path pathToBucket = new Path(pathToVolume, bucketName);
         final Path pathToTrash = new Path(pathToBucket, TRASH_PREFIX);
         return new Path(pathToTrash, username);
       } catch (IOException ex) {
         throw new RuntimeException("getTrashRoot failed.", ex);
       }
     }
   
   @smengcl can you please confirm why we have volumename and bucketname in the trash path ?




-- 
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] github-actions[bot] closed pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #2747:
URL: https://github.com/apache/ozone/pull/2747


   


-- 
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 #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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


   /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] sadanand48 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       I have done this only to obtain the keyName from the  dst. 
   value of dst  would be something like : `/volume08102/bucket23262/.Trash/sadanand.shenoy/Current/volume08102/bucket23262/keyToBeDeleted`




-- 
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] sadanand48 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       @mukul1987 problem lies [here](https://github.com/apache/hadoop/blob/616cea2e8068e990d24057d2b0d6090f35e21371/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java#L145 ) where in the destination path is getting constructed.
   ` } else {
           Path trashPath = this.makeTrashRelativePath(trashCurrent, path);`
   
   `trashCurrent ` for  o3fs : /.Trash/sadanand.shenoy/Current
   ` trashCurrent` for ofs : ofs:/volume22368/bucket38935/.Trash/sadanand.shenoy/Current
   
   `path` for o3fs:  keyToBeDeleted
   `path `for ofs: /volume22368/bucket38935/keyToBeDeleted
   
   For ofs , the `path` variable includes the volume and bucket name .




-- 
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] smengcl commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);
+      String keyName = dstpath.getKeyName();
+      // omit volume name and bucket name from Key
+      String keyNamewithoutVolumeAndBucket = keyName.replace(

Review comment:
       `String.replace()` wouldn't work as expected in some edge cases.
   
   e.g.  when vol/bucket is repeated in the String: `keyName = /vol1/buck2/vol1/buck2/key1` becomes `/key1` after replacement with your current logic. But we actually expect `/vol1/buck2/key1` in this specific case here, right?
   
   `String.replace()` replaces ALL occurrences of the pattern. But what we actually want is just the **first** occurrence from the **beginning** of the String.
   
   I think we can just parse `keyName` into `OFSPath` and use the existing method `OFSPath#getKeyName` to get the proper key path.




-- 
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] smengcl commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);
+      String keyName = dstpath.getKeyName();
+      // omit volume name and bucket name from Key
+      String keyNamewithoutVolumeAndBucket = keyName.replace(

Review comment:
       `String.replace()` wouldn't work as expected in some edge cases.
   
   e.g.  when vol/bucket is repeated in the String: `keyName = /vol1/buck2/vol1/buck2/key1` becomes `/key1` after replacement with your current logic. But we actually expect `/vol1/buck2/key1` in this specific case here, right?
   
   `String.replace()` replaces ALL occurrences of the pattern. But what we actually want is just the **first** occurrence from the **beginning** of the String.
   
   I think we can just parse `keyName` into `OFSPath` and use the existing method `OFSPath#getKeyName`.




-- 
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] smengcl commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       @mukul1987
   
   @sadanand48 is right. The `path` passed to `makeTrashRelativePath` contains volume and bucket in OFS currently.




-- 
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] mukul1987 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       How is this name constructed on the first go ?

##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -447,7 +458,7 @@ protected void rename(final Path src, final Path dst,
       super(f);
       this.recursive = recursive;
       if (getStatus().isDirectory()
-          && !this.recursive
+          && !this. recursive

Review comment:
       This change is not needed.




-- 
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] mukul1987 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       I guess the problem is in
   `
     public Path getTrashRoot() {
       if (!this.isKey()) {
         throw new RuntimeException("Volume or bucket doesn't have trash root.");
       }
       try {
         final String username =
                 UserGroupInformation.getCurrentUser().getShortUserName();
         final Path pathRoot = new Path(
             OZONE_OFS_URI_SCHEME, authority, OZONE_URI_DELIMITER);
         final Path pathToVolume = new Path(pathRoot, volumeName);
         final Path pathToBucket = new Path(pathToVolume, bucketName);
         final Path pathToTrash = new Path(pathToBucket, TRASH_PREFIX);
         return new Path(pathToTrash, username);
       } catch (IOException ex) {
         throw new RuntimeException("getTrashRoot failed.", ex);
       }
     }
   `
   @smengcl can you please confirm why we have volumename and bucketname in the trash path ?




-- 
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] smengcl commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);
+      String keyName = dstpath.getKeyName();
+      // omit volume name and bucket name from Key
+      String keyNamewithoutVolumeAndBucket = keyName.replace(

Review comment:
       `String.replace()` wouldn't work as expected in some edge cases.
   
   e.g.  when vol/bucket is repeated in the String: `keyName = /vol1/buck2/vol1/buck2/key1` becomes `/key1` after replacement with your current logic. But we actually expect `/vol1/buck2/key1` in this specific case here, right?
   
   `String.replace()` replaces ALL occurrences of the pattern. But what we actually want is just the **first** occurrence from the **beginning** of the String.




-- 
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] smengcl commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);
+      String keyName = dstpath.getKeyName();
+      // omit volume name and bucket name from Key
+      String keyNamewithoutVolumeAndBucket = keyName.replace(

Review comment:
       Hmm. Why is this marked as resolved without comments or code change?




-- 
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] sadanand48 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       @mukul1987 problem lies [here](https://github.com/apache/hadoop/blob/616cea2e8068e990d24057d2b0d6090f35e21371/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java#L145 ) where in the destination path is getting constructed.
   ` } else {
           Path trashPath = this.makeTrashRelativePath(trashCurrent, path);`
   
   `trashCurrent ` for  o3fs : /.Trash/sadanand.shenoy/Current
   ` trashCurrent` for ofs : ofs:/volume22368/bucket38935/.Trash/sadanand.shenoy/Current
   
   `path` for o3fs:  keyToBeDeleted
   `path `for ofs: /volume22368/bucket38935/keyToBeDeleted
   
   For ofs , the `path` variable includes the volume and bucket name .
   One way this could be solved is by overriding `movetoTrash(Path path)` in TrashPolicyOzone.java but that would require duplicating around 70 lines of code.




-- 
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] sadanand48 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);

Review comment:
       @mukul1987 problem lies [here](https://github.com/apache/hadoop/blob/616cea2e8068e990d24057d2b0d6090f35e21371/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashPolicyDefault.java#L145 )
   ` } else {
           Path trashPath = this.makeTrashRelativePath(trashCurrent, path);`
   
   `trashCurrent ` for  o3fs : /.Trash/sadanand.shenoy/Current
   ` trashCurrent` for ofs : ofs:/volume22368/bucket38935/.Trash/sadanand.shenoy/Current
   
   `path` for o3fs:  keyToBeDeleted
   `path `for ofs: /volume22368/bucket38935/keyToBeDeleted
   
   For ofs , the `path` variable includes the volume and bucket name .




-- 
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] smengcl commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -433,7 +433,18 @@ protected void rename(final Path src, final Path dst,
       // if doesn't have TO_TRASH option, just pass the call to super
       super.rename(src, dst, options);
     } else {
-      rename(src, dst);
+      OFSPath dstpath = new OFSPath(dst);
+      String keyName = dstpath.getKeyName();
+      // omit volume name and bucket name from Key
+      String keyNamewithoutVolumeAndBucket = keyName.replace(

Review comment:
       `String.replace()` wouldn't work as expected in some edge cases.
   
   e.g.  when vol/bucket is repeated in the String: `keyName = /vol1/buck2/vol1/buck2/key1` becomes `/key1` after replacement with your current logic. But we actually expect `/vol1/buck2/key1` here, right?
   
   `String.replace()` replaces ALL occurrences of the pattern. But what we actually want is just the **first** occurrence from the **beginning** of the String.




-- 
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] sadanand48 commented on a change in pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

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



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -447,7 +458,7 @@ protected void rename(final Path src, final Path dst,
       super(f);
       this.recursive = recursive;
       if (getStatus().isDirectory()
-          && !this.recursive
+          && !this. recursive

Review comment:
       done




-- 
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] github-actions[bot] commented on pull request #2747: HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #2747:
URL: https://github.com/apache/ozone/pull/2747#issuecomment-1043663861


   Thank you very much for the patch. I am closing this PR __temporarily__ as there was no activity recently and it is waiting for response from its author.
   
   It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.
   
   It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.
   
   If you need ANY help to finish this PR, please [contact the community](https://github.com/apache/hadoop-ozone#contact) on the mailing list or the slack channel."


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