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 2020/12/07 06:20:49 UTC
[GitHub] [ozone] mukul1987 opened a new pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
mukul1987 opened a new pull request #1665:
URL: https://github.com/apache/ozone/pull/1665
## What changes were proposed in this pull request?
During Ozone Key, File, and Directory creations, intermediate directories are not counted towards the number of keys in the system. When a directory is deleted, however, we traverse each of the keys and delete it, this results in underflow of the numKey metric.
## What is the link to the Apache JIRA
(Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
Please replace this section with the link to the Apache JIRA)
https://issues.apache.org/jira/browse/HDDS-4554
(Please explain how this patch was tested. Ex: unit tests, manual tests)
Added a new 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.
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] xiaoyuyao commented on a change in pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#discussion_r538602633
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -336,6 +338,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
yes. Thanks @bharatviswa504
----------------------------------------------------------------
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] xiaoyuyao commented on a change in pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#discussion_r538605851
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -336,6 +338,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
Thanks for the detail. Makes sense to me. Can we add some comments on this?
----------------------------------------------------------------
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] ayushtkn commented on pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#issuecomment-740379836
Thanx Mukul for the fix.
`TestOMDirectoryCreateRequest.testCreateDirectoryOMMetric` looks related, I think you need to increment the value there in the test to include the missing directories seems related, Apart changes looks reasonable
>The key/file value is incremented because of metric increment in OmKeyCommitRequest
Thnax for this I too spent around an hour trying to figure out the logic behind this yesterday. :-)
----------------------------------------------------------------
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] mukul1987 merged pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
mukul1987 merged pull request #1665:
URL: https://github.com/apache/ozone/pull/1665
----------------------------------------------------------------
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] mukul1987 commented on pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#issuecomment-741515283
@ayushtkn @xiaoyuyao @bharatviswa504 I have addressed the review comments for the patch. Please have a look at the patch.
----------------------------------------------------------------
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] mukul1987 commented on a change in pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#discussion_r537717513
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -345,6 +346,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
Same as the previous discussion
----------------------------------------------------------------
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] bharatviswa504 commented on a change in pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#discussion_r537689768
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -336,6 +338,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
This is already taken care
```
} catch (IOException ex) {
result = Result.FAILURE;
exception = ex;
omMetrics.incNumCreateFileFails();
```
Is this what you are looking for?
----------------------------------------------------------------
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] mukul1987 commented on pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#issuecomment-741657217
Thanks for the reviews @ayushtkn @xiaoyuyao and @bharatviswa504. I have merged this 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.
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] xiaoyuyao commented on a change in pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#discussion_r537653928
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -336,6 +338,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
We need to add incNumCreateFileFails for the failure case below?
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemMetrics.java
##########
@@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.ozone;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.junit.*;
Review comment:
NIT: can we expand the wildcard import?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -336,6 +338,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
Should this be numMissingParents +1 ?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -345,6 +346,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
Same as above.
----------------------------------------------------------------
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] mukul1987 commented on a change in pull request #1665: HDDS-4554. numKey metrics goes negative after intermediate directory deletion.
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #1665:
URL: https://github.com/apache/ozone/pull/1665#discussion_r537717407
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -336,6 +338,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
switch (result) {
case SUCCESS:
+ omMetrics.incNumKeys(numMissingParents);
Review comment:
The metric being incremented here is because of the missing directories. The key/file value is incremented because of metric increment in OmKeyCommitRequest
----------------------------------------------------------------
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