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/10/17 16:24:08 UTC
[GitHub] [ozone] aswinshakil opened a new pull request, #3851: HDDS-6210. EC: Add EC metrics
aswinshakil opened a new pull request, #3851:
URL: https://github.com/apache/ozone/pull/3851
## What changes were proposed in this pull request?
To add EC metrics for OM, DN, and Client side.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6210
## How was this patch tested?
Unit tests and 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] sodonnel merged pull request #3851: HDDS-6210. EC: Add EC metrics
Posted by GitBox <gi...@apache.org>.
sodonnel merged PR #3851:
URL: https://github.com/apache/ozone/pull/3851
--
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] sodonnel commented on a diff in pull request #3851: HDDS-6210. EC: Add EC metrics
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3851:
URL: https://github.com/apache/ozone/pull/3851#discussion_r999273782
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java:
##########
@@ -157,11 +158,17 @@ public synchronized int read(ByteBuffer buf) throws IOException {
lastPosition = blockReader.getPos();
totalRead += blockReader.read(buf);
}
+ if (reconstructionReader) {
Review Comment:
I wonder if this makes sense? This read method is going to be called for every read into a file. Eg if something is reading a file 1 byte at a time, it could be called 100's of times. Would it make more sense to increment just once when the proxy class sets up the reconstruction reader? Eg in `createBlockReader()` in the same class?
--
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] aswinshakil commented on a diff in pull request #3851: HDDS-6210. EC: Add EC metrics
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on code in PR #3851:
URL: https://github.com/apache/ozone/pull/3851#discussion_r998433536
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionMetrics.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.ozone.container.ec.reconstruction;
+
+import org.apache.hadoop.hdds.annotation.InterfaceAudience;
+import org.apache.hadoop.metrics2.MetricsSystem;
+import org.apache.hadoop.metrics2.annotation.Metric;
+import org.apache.hadoop.metrics2.annotation.Metrics;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.apache.hadoop.metrics2.lib.MutableCounterLong;
+import org.apache.hadoop.ozone.OzoneConsts;
+
+/**
+ * Metrics class for EC Reconstruction.
+ */
+@InterfaceAudience.Private
+@Metrics(about = "EC Reconstruction Coordinator Metrics",
+ context = OzoneConsts.OZONE)
+public final class ECReconstructionMetrics {
+ private static final String SOURCE =
+ ECReconstructionMetrics.class.getSimpleName();
+
+ private @Metric MutableCounterLong numBlockGroupReconstruction;
Review Comment:
Thanks for the review. Will update the PR.
--
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] sodonnel commented on a diff in pull request #3851: HDDS-6210. EC: Add EC metrics
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3851:
URL: https://github.com/apache/ozone/pull/3851#discussion_r998007891
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionMetrics.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.ozone.container.ec.reconstruction;
+
+import org.apache.hadoop.hdds.annotation.InterfaceAudience;
+import org.apache.hadoop.metrics2.MetricsSystem;
+import org.apache.hadoop.metrics2.annotation.Metric;
+import org.apache.hadoop.metrics2.annotation.Metrics;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.apache.hadoop.metrics2.lib.MutableCounterLong;
+import org.apache.hadoop.ozone.OzoneConsts;
+
+/**
+ * Metrics class for EC Reconstruction.
+ */
+@InterfaceAudience.Private
+@Metrics(about = "EC Reconstruction Coordinator Metrics",
+ context = OzoneConsts.OZONE)
+public final class ECReconstructionMetrics {
+ private static final String SOURCE =
+ ECReconstructionMetrics.class.getSimpleName();
+
+ private @Metric MutableCounterLong numBlockGroupReconstruction;
Review Comment:
@kerneltime Commented in https://github.com/apache/ozone/pull/3781#discussion_r984819741 that we should name metrics with the most descriptive part of the name first, so you can filter the metric more easily. Ie, instead of `numBlockGroupReconstruction` we should have `BlockGroupReconstructionNum` or maybe `BlockGroupReconstructionTotal`.
--
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] aswinshakil commented on a diff in pull request #3851: HDDS-6210. EC: Add EC metrics
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on code in PR #3851:
URL: https://github.com/apache/ozone/pull/3851#discussion_r999725898
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java:
##########
@@ -157,11 +158,17 @@ public synchronized int read(ByteBuffer buf) throws IOException {
lastPosition = blockReader.getPos();
totalRead += blockReader.read(buf);
}
+ if (reconstructionReader) {
Review Comment:
Makes sense. Thanks for pointing out, Will update the PR
--
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