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

[GitHub] [ozone] tanvipenumudy opened a new pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

tanvipenumudy opened a new pull request #3077:
URL: https://github.com/apache/ozone/pull/3077


   ## What changes were proposed in this pull request?
   
   Added `numFSOBucketCreates` metrics to reflect the number of FSO bucket creation requests.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6305
   
   ## How was this patch tested?
   
   Existing unit 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] ayushtkn commented on a change in pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       There is a ``bucketInfo.getBucketLayout() == null`` in the above if block, if that comes true, your code will fetch a NPE here.
   
   Not very sure if it would be ``null`` in any case or not. It should have defaulted to ``LEGACY`` in case of ``null``, May be added for some upgrade stuff or just for sanity...
   
   You can explore pulling up the check inside the else block, that will skip checking ``isFileOptimised`` when you already know the bucket is of type LEGACY. 
   
   ```
       } else {
         omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
         if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
           omMetrics.incNumFSOBucketCreates();
         }
   ```




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
##########
@@ -306,6 +307,10 @@ public void incNumBucketCreates() {
     numBucketCreates.incr();
   }
 
+  public void incNumFSOBucketCreates() {
+    numFSOBucketCreates.incr();

Review comment:
       Yeah looks like I missed that, I don't think we need to update that twice, please ignore my comment.




-- 
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] ayushtkn commented on a change in pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       Hey Rakesh,
   Got little confused, you mean to say:
   ```
       if (bucketInfo.getBucketLayout() == null || bucketInfo.getBucketLayout()
           .equals(BucketLayoutProto.LEGACY)) {
         // Bucket Layout argument was not passed during bucket creation.
         String omDefaultBucketLayout = ozoneManager.getOMDefaultBucketLayout();
         BucketLayout defaultType = BucketLayout.fromString(omDefaultBucketLayout);
         omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo, defaultType);
       } else {
         omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
       }
   ```
   Post this block there won't be any ``null`` check required because ``omBucketInfo`` is being set to some value in this block?
   Then in that case value is being set to ``omBucketInfo`` and in the new code added there is an operation ``omBucketInfo.getBucketLayout().isFileSystemOptimized()``, we didn't modify ``omBucketInfo.getBucketLayout()``? So if that is ``null`` and we call for ``isFileSystemOptimized()`` on top of it, this would throw NPE, right?
   
   Or if you mean to say pull the new code in the else block, yeps that won't require any null check.
   
   Just had a quick look on it, might have messed up. Please don't hold this for me. You can proceed if you are convinced with the present 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] ayushtkn commented on a change in pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       Cool 




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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


   Thanks for the contribution @tanvipenumudy! I have added one comment around the metrics. Please check that out.
   
   There seems to be an issue with the formatting, please run `./checkstyle.sh` script present in `ozone/hadoop-ozone/dev-support/checks` directory to find the issues and correct them.


-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       Hey Ayush,
   ```java
   BucketLayout defaultType = BucketLayout.fromString(omDefaultBucketLayout);
   omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo, defaultType);
   ```
   These lines inside the `if` condition will set the bucketLayout for omBucketInfo in case it is not present - so it will not be null. 
   The second argument in `OmBucketInfo.getFromProtobuf` call is the bucket layout being passed.




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
##########
@@ -306,6 +307,10 @@ public void incNumBucketCreates() {
     numBucketCreates.incr();
   }
 
+  public void incNumFSOBucketCreates() {
+    numFSOBucketCreates.incr();

Review comment:
       We might want to update the `numBucketOps` here as well?
   ```
   numBucketOps.incr();
   ```




-- 
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] ayushtkn commented on a change in pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       There is a ``bucketInfo.getBucketLayout() == null`` in the above if block, if that comes true, your code will fetch a NPE here.
   
   Not very sure if it would be ``null`` in any case or not. It should have defaulted to ``LEGACY`` in case of ``null``, May be added for some upgrade stuff or just for sanity...
   
   You can explore pulling up the check inside the else block, that will skip checking ``isFileOptimised`` when you already know the bucket is of type LEGACY. 
   
   ```
       } else {
         omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
         if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
           omMetrics.incNumFSOBucketCreates();
         }
   ```




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       Hey Ayush,
   ```java
   BucketLayout defaultType = BucketLayout.fromString(omDefaultBucketLayout);
   omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo, defaultType);
   ```
   These lines inside the `if` condition will set the bucketLayout for omBucketInfo to `OBJECT_STORE` or `FILE_SYSTEM_OPTIMIZED` in case it is not present - so it will not be null. 
   The second argument in `OmBucketInfo.getFromProtobuf` call is the bucket layout being passed.




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       The null check is added only for testing purposes. It will never be null in real-world scenarios. We could however move the metrics-related code into the if-else block in case we are running into any NPEs during testing.




-- 
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 edited a comment on pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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


   Thanks @tanvipenumudy for the contribution.
   
   Thanks @ayushtkn @siddhantsangwan @JyotinderSingh for the reviews.
   
   +1LGTM, Pending CI report.


-- 
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] siddhantsangwan commented on a change in pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
##########
@@ -306,6 +307,10 @@ public void incNumBucketCreates() {
     numBucketCreates.incr();
   }
 
+  public void incNumFSOBucketCreates() {
+    numFSOBucketCreates.incr();

Review comment:
       Looks like `numBucketOps` also gets incremented in `OMMetrics#incNumBucketCreates()`, which is called earlier at https://github.com/apache/ozone/blob/7c990abc85d4142e1c30563cd67dc601f170eb90/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java#L129
   
   Do we want to increment `numBucketOps` twice?




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       Hey Ayush,
   ```java
   BucketLayout defaultType = BucketLayout.fromString(omDefaultBucketLayout);
   omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo, defaultType);
   ```
   These lines inside the `if` condition will set the bucketLayout for omBucketInfo to `OBJECT_STORE` or `FILE_SYSTEM_OPTIMIZED` in case it is not present - so it will not be null. 
   
   (The second argument in `OmBucketInfo.getFromProtobuf` call is the bucket layout being passed)




-- 
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 pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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


   Thanks @tanvipenumudy for the contribution.
   
   Thanks @ayushtkn @siddhantsangwan @JyotinderSingh for the reviews.
   
   +1LGTM, will merge it shortly.


-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       @ayushtkn @JyotinderSingh 
   
   ```
    if (bucketInfo.getBucketLayout() == null)
          // sets to defaultValue. Here it can be OBS or FSO.
   
    } else {
         omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
    }
   ```
   
   Since there is already an explicit `null` check [OMBucketCreateRequest.java#L142](https://github.com/apache/ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java#L142) to safeguard NPE, I think it won't be null in any case and additional null check is not required, isn't 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.

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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       The null check is added only for testing purposes. It will never be null in real-world scenarios. We could however move the metrics-related code into the else block in case we are running into any NPEs.




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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


   


-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       The null check is added only for testing purposes. It will never be null in real-world scenarios. We could however move the metrics-related code into the else block in case we are running into any NPEs.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       The null check is added only for testing purposes. It will never be null in real-world scenarios. We could however move the metrics-related code into the if-else block in case we are running into any NPEs.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       The null check is added only for testing purposes. It will never be null in real-world scenarios. We could however move the metrics-related code into the if-else block in case we are running into any NPEs during testing.




-- 
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 #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       The null check is added only for testing purposes. It will never be null in real-world scenarios. We could however move the metrics-related code into the if-else block in case we are running into any NPEs.




-- 
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] ayushtkn commented on a change in pull request #3077: HDDS-6305: Add metrics - number of FSO bucket creates

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -149,7 +149,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     } else {
       omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo);
     }
-
+    if (omBucketInfo.getBucketLayout().isFileSystemOptimized()) {
+      omMetrics.incNumFSOBucketCreates();
+    }

Review comment:
       Cool got messed up bucketInfo vs omBucketInfo




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