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

[PR] HDDS-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   ## What changes were proposed in this pull request?
   
   `BucketEndPoint` returns keys without considering `"/" ` for FSO bucket, So it matches `dir1/` with `dir123` also, and return back the result. Because of this it leads to incorrect behaviour for FSO bucket keys.
    To fix this issue in this PR we are ensuring exact prefix match so that for FSO bucket result from `BucketEndPoint` is proper.
   
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-9229
   
   ## How was this patch tested?
   
   Manually tested using dfs s3a for FSO bucket.
   


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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


##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -206,6 +207,13 @@ public Response get(
     int count = 0;
     while (ozoneKeyIterator.hasNext()) {
       OzoneKey next = ozoneKeyIterator.next();
+      if (bucket.getBucketLayout().isFileSystemOptimized() &&
+          StringUtils.isNotEmpty(prefix) &&
+          !next.getName().contains(prefix)) {

Review Comment:
   It's better to use startWith instead of contains here. 



-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   > Could you add a test for this which reproduces the original issue just on the Ozone side? Ie a test which simulates what S3A does to result in the incorrect listing?
   
   @sodonnel Thanks for the review.
   
   Below is the process that leads to the problem:
   
   Operation: Create key `word.txt` from dfs s3a in fso bucket.
   dfs with s3a creates a temp key first, then copy temp key to actual key.
   
   Here are the operations that happens at Ozone:
     i)  Call received at `BucketEndPoint(get)` with prefix `word.txt/ ` --> return empty response from ozone as no such key exist
    ii)  Call received at `BucketEndPoint(get)` with prefix `word.txt._COPYING_/`  --> return empty response from ozone as no such key exist
    iii) Call received at ObjectEndPoint(put) with keyPath= `word.txt_.COPYING_` with the actual content. -->In ozone Key gets created with name `word.txt_.COPYING_`
    iv)  Call received at `BucketEndPoint(get)` with prefix `word.txt/ `Ozone returns `word.txt._COPYING_ `back to s3a
    v)   Call received at `ObjectEndPoint(put)` with keyPath=`word.txt/ord.txt._COPYING_` and  header includes x-amz-copy-source"
   Now for ozone in case of FSO there are two keys a) word.txt/ b) ord.txt._COPYING_ and both doesn't exist.
   So two keys are getting created and the content is getting copied into `word.txt/ord.txt._COPYING_`
   
   Looking at step (iv) since ozone has returned key with the prefix exist, s3a is appending that key name with the actual key and sending copy put object at step(v).
   
   Problem is happening at step(iv) which is about listing, You want to have test on that?


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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


##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java:
##########
@@ -206,6 +207,13 @@ public Response get(
     int count = 0;
     while (ozoneKeyIterator.hasNext()) {
       OzoneKey next = ozoneKeyIterator.next();
+      if (bucket.getBucketLayout().isFileSystemOptimized() &&
+          StringUtils.isNotEmpty(prefix) &&
+          !next.getName().contains(prefix)) {

Review Comment:
   @ChenSammi Thanks for the review, Changed to `startsWith`.



-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   I will try to add acceptance test to simulate this behaviour.


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   @sodonnel I have updated my previous [comment](https://github.com/apache/ozone/pull/5422#issuecomment-1758880474) to describe why this problem is happening.
   I have added test to simulate s3a behaviour in ozone.
   Can you please look and provide your inputs.


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   @SaketaChalamchala @xBis7 can you'll please take a look.


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   Could you add a test for this which reproduces the original issue just on the Ozone side? Ie a test which simulates what S3A does to result in the incorrect listing?


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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

   I was thinking a test on the part of Ozone that gives the problem to S3A. An acceptance test probably isn't necessary. Its hard for me to understand how the fix in this PR corrects anything in Ozone without a test to show the broken / expected behavior.


-- 
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-9229. Fix S3A compatibility - dfs -put for FSO bucket. [ozone]

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


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