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/23 16:26:53 UTC

[GitHub] [ozone] JyotinderSingh opened a new pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

JyotinderSingh opened a new pull request #2867:
URL: https://github.com/apache/ozone/pull/2867


   …ource bucket.
   
   ## What changes were proposed in this pull request?
   
   This bug was discovered while working on HDDS-5959.
   buckets created via 
   
   ```bash
   ozone sh bucket link <source bucket> <new bucket>
   ```
    do not replicate the bucket layout of the source bucket, and instead fallback to the default bucket layout defined via 
   ```
   ozone.default.bucket.layout
   ```
   
   ![image](https://issues.apache.org/jira/secure/attachment/13036502/13036502_Screenshot+2021-11-17+at+4.10.45+PM.png)
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6043
   
   ## How was this patch tested?
   
   unit + integration tests
   


-- 
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] JyotinderSingh commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -115,6 +115,22 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
           OMException.ResultCodes.INVALID_REQUEST);
     }
 
+    // If this is a Link Bucket, we make sure that its layout matches the
+    //  source bucket. This prevents inconsistencies during list operations.
+    if (hasSourceBucket) {
+      try {
+        newBucketInfo.setBucketLayout(

Review comment:
       I have added this in `5ab6ce4`. Currently the fix is only implemented for RpcClient#listBuckets. I'm working to get it working for OmMetadataManagerImpl#listBuckets.




-- 
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] JyotinderSingh commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -785,24 +786,24 @@ public OzoneBucket getBucketDetails(
         volumeName, prevBucket, bucketPrefix, maxListResult);
 
     return buckets.stream().map(bucket -> new OzoneBucket(
-        conf,

Review comment:
       Yes, it must have been introduced during an intermediate change. I'll revert this. Thanks!




-- 
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] JyotinderSingh commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -755,6 +756,20 @@ public OzoneBucket getBucketDetails(
     verifyBucketName(bucketName);
     OmBucketInfo bucketInfo =
         ozoneManagerClient.getBucketInfo(volumeName, bucketName);
+
+    BucketLayout bucketLayout = bucketInfo.getBucketLayout();

Review comment:
       Thanks for the suggestion @rakeshadr. I have removed the listBuckets modification for Link Buckets.




-- 
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] rakeshadr commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -785,24 +786,24 @@ public OzoneBucket getBucketDetails(
         volumeName, prevBucket, bucketPrefix, maxListResult);
 
     return buckets.stream().map(bucket -> new OzoneBucket(
-        conf,

Review comment:
       @JyotinderSingh can we revert RPCclient.java changes. It looks to me there is no change done except indentation. 




-- 
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] rakeshadr commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -755,6 +756,20 @@ public OzoneBucket getBucketDetails(
     verifyBucketName(bucketName);
     OmBucketInfo bucketInfo =
         ozoneManagerClient.getBucketInfo(volumeName, bucketName);
+
+    BucketLayout bucketLayout = bucketInfo.getBucketLayout();

Review comment:
       @JyotinderSingh IMHO listing `#linkBucket ` implementation can be moved out to a separate jira. Apart from the list operation the patch looks good to me and those changes are enough to unblock the long waiting patch HDDS-5959, which has to be pushed in asap to unblock all the follow up tasks. Please create a new sub-task to implement the listing of linkBucket content and rebase this patch to include only the other changes. Does that sound good?




-- 
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] rakeshadr merged pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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


   


-- 
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] JyotinderSingh commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -115,6 +115,22 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
           OMException.ResultCodes.INVALID_REQUEST);
     }
 
+    // If this is a Link Bucket, we make sure that its layout matches the
+    //  source bucket. This prevents inconsistencies during list operations.
+    if (hasSourceBucket) {
+      try {
+        newBucketInfo.setBucketLayout(

Review comment:
       I have added this in `a572156`. Currently the fix is only implemented for RpcClient#listBuckets. I'm working to get it working for OmMetadataManagerImpl#listBuckets.




-- 
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] JyotinderSingh commented on pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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


   Thanks for the review @bharatviswa504, I am adding an update to the patch which will make sure the layout is fetched from the source bucket - both during getBucketLayout operations and during link bucket creation.


-- 
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] rakeshadr commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -115,6 +115,22 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
           OMException.ResultCodes.INVALID_REQUEST);
     }
 
+    // If this is a Link Bucket, we make sure that its layout matches the
+    //  source bucket. This prevents inconsistencies during list operations.
+    if (hasSourceBucket) {
+      try {
+        newBucketInfo.setBucketLayout(

Review comment:
       Still there is one gap for the pre-created bucket-links. Those buckets will mismatched with the real source bucket. Instead of adding a logic here, how about checking `omBucketInfo#isLink()` during listing bucket then get the source bucket's layout, if source exists?
   
   https://github.com/apache/ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java#L826




-- 
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] rakeshadr commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -755,6 +756,20 @@ public OzoneBucket getBucketDetails(
     verifyBucketName(bucketName);
     OmBucketInfo bucketInfo =
         ozoneManagerClient.getBucketInfo(volumeName, bucketName);
+
+    BucketLayout bucketLayout = bucketInfo.getBucketLayout();

Review comment:
       @JyotinderSingh IMHO listing `#linkBucket ` implementation can be moved out to a separate jira. Apart from the list operation the patch looks good to me and those changes are enough to unblock the long waiting patch HDDS-5959, which has to be pushed in asap to unblock all the follow up tasks Please create a new sub-task to implement the listing of linkBucket content and rebase this patch to include only the other changes. Does that sound good?




-- 
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] JyotinderSingh commented on a change in pull request #2867: HDDS-6043. Buckets created via link command do not mirror layout of s…

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -115,6 +115,22 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
           OMException.ResultCodes.INVALID_REQUEST);
     }
 
+    // If this is a Link Bucket, we make sure that its layout matches the
+    //  source bucket. This prevents inconsistencies during list operations.
+    if (hasSourceBucket) {
+      try {
+        newBucketInfo.setBucketLayout(

Review comment:
       I have added this in `6b1980b`. Currently the fix is only implemented for RpcClient#listBuckets. I'm working to get it working for OmMetadataManagerImpl#listBuckets.




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