[
https://issues.apache.org/jira/browse/HADOOP-19311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17906254#comment-17906254
]
ASF GitHub Bot commented on HADOOP-19311:
-----------------------------------------
anujmodi2021 commented on code in PR #7122:
URL: https://github.com/apache/hadoop/pull/7122#discussion_r1887862408
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/enums/RetryValue.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
+ *
+ * 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.azurebfs.enums;
+
+/**
+ * Enum for retry values.
Review Comment:
We can also add where these are used, i.e. Metrics
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/enums/AbfsBackoffMetricsEnum.java:
##########
@@ -0,0 +1,100 @@
+/**
+ * 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.azurebfs.enums;
+
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.BASE;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.RETRY;
+import static
org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
+import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_GAUGE;
+
+/**
+ * Enum representing various ABFS backoff metrics
+ */
+public enum AbfsBackoffMetricsEnum {
+ NUMBER_OF_IOPS_THROTTLED_REQUESTS("numberOfIOPSThrottledRequests", "Number
of IOPS throttled requests", BASE, TYPE_COUNTER),
Review Comment:
Line should not be longer than 80-100 characters for better readability.
This needs to be checked everywhere.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBackoffMetrics.java:
##########
@@ -0,0 +1,275 @@
+/**
+ * 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.azurebfs.services;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum;
+import org.apache.hadoop.fs.azurebfs.enums.RetryValue;
+import org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum;
+import org.apache.hadoop.fs.azurebfs.statistics.AbstractAbfsStatisticsSource;
+import org.apache.hadoop.fs.statistics.impl.IOStatisticsStore;
+
+import static
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EMPTY_STRING;
+import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.THOUSAND;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.RETRY;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.COLON;
+import static
org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.DOUBLE_PRECISION_FORMAT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MIN_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_RETRY_COUNT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_BANDWIDTH_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_IOPS_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_NETWORK_FAILED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_OTHER_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_FAILED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED_WITHOUT_RETRYING;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.ONE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWO;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.THREE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FOUR;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIVE_FIFTEEN;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIFTEEN_TWENTY_FIVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWENTY_FIVE_AND_ABOVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
+import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_GAUGE;
+import static
org.apache.hadoop.fs.statistics.impl.IOStatisticsBinding.iostatisticsStore;
+import static org.apache.hadoop.util.StringUtils.format;
+import static org.apache.hadoop.util.StringUtils.formatPercent;
+
+/**
+ * This class is responsible for tracking and
+ * updating metrics related to backoff and
+ * retry operations in Azure Blob File System (ABFS).
+ */
+public class AbfsBackoffMetrics extends AbstractAbfsStatisticsSource {
+ private static final List<RetryValue> RETRY_LIST = Arrays.asList(ONE, TWO,
THREE, FOUR, FIVE_FIFTEEN, FIFTEEN_TWENTY_FIVE, TWENTY_FIVE_AND_ABOVE);
+
+ /**
+ * Constructor to initialize the IOStatisticsStore with counters and gauges.
+ */
+ public AbfsBackoffMetrics() {
+ IOStatisticsStore ioStatisticsStore = iostatisticsStore()
+ .withCounters(getMetricNames(TYPE_COUNTER))
+ .withGauges(getMetricNames(TYPE_GAUGE))
+ .build();
+ setIOStatistics(ioStatisticsStore);
+ }
+
+ /**
+ * Retrieves the metric names based on the statistic type.
+ *
+ * @param type the type of the statistic (counter or gauge)
+ * @return an array of metric names
+ */
+ private String[] getMetricNames(StatisticTypeEnum type) {
+ return Arrays.stream(AbfsBackoffMetricsEnum.values())
+ .filter(backoffMetricsEnum ->
backoffMetricsEnum.getStatisticType().equals(type))
+ .flatMap(backoffMetricsEnum ->
+ RETRY.equals(backoffMetricsEnum.getType())
+ ? RETRY_LIST.stream().map(retryCount ->
retryCount.getValue() + COLON + backoffMetricsEnum.getName())
+ : Stream.of(backoffMetricsEnum.getName())
+ ).toArray(String[]::new);
+ }
+
+ /**
+ * Constructs the metric name based on the metric and retry value.
+ *
+ * @param metric the metric enum
+ * @param retryValue the retry value
+ * @return the constructed metric name
+ */
+ private String getMetricName(AbfsBackoffMetricsEnum metric, RetryValue
retryValue) {
+ if (RETRY.equals(metric.getType())) {
+ return retryValue.getValue() + COLON + metric.getName();
+ }
+ return metric.getName();
+ }
+
+ /**
+ * Retrieves the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param retryValue the retry value
+ * @return the value of the metric
+ */
+ public long getMetricValue(AbfsBackoffMetricsEnum metric, RetryValue
retryValue) {
+ String metricName = getMetricName(metric, retryValue);
+ switch (metric.getStatisticType()) {
+ case TYPE_COUNTER:
+ return lookupCounterValue(metricName);
+ case TYPE_GAUGE:
+ return lookupGaugeValue(metricName);
+ default:
+ return 0;
+ }
+ }
+
+ /**
+ * Retrieves the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @return the value of the metric
+ */
+ public long getMetricValue(AbfsBackoffMetricsEnum metric) {
+ return getMetricValue(metric, null);
+ }
+
+ /**
+ * Increments the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param retryValue the retry value
+ */
+ public void incrementMetricValue(AbfsBackoffMetricsEnum metric, RetryValue
retryValue) {
+ String metricName = getMetricName(metric, retryValue);
+ switch (metric.getStatisticType()) {
+ case TYPE_COUNTER:
+ incCounterValue(metricName);
+ break;
+ case TYPE_GAUGE:
+ incGaugeValue(metricName);
+ break;
+ default:
+ // Do nothing
+ break;
+ }
+ }
+
+ /**
+ * Increments the value of a specific metric.
+ *
+ * @param metric the metric enum
+ */
+ public void incrementMetricValue(AbfsBackoffMetricsEnum metric) {
+ incrementMetricValue(metric, null);
+ }
+
+ /**
+ * Sets the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param value the new value of the metric
+ * @param retryValue the retry value
+ */
+ public void setMetricValue(AbfsBackoffMetricsEnum metric, long value,
RetryValue retryValue) {
+ String metricName = getMetricName(metric, retryValue);
+ switch (metric.getStatisticType()) {
+ case TYPE_COUNTER:
+ setCounterValue(metricName, value);
+ break;
+ case TYPE_GAUGE:
+ setGaugeValue(metricName, value);
+ break;
+ default:
+ // Do nothing
+ break;
+ }
+ }
+
+ /**
+ * Sets the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param value the new value of the metric
+ */
+ public void setMetricValue(AbfsBackoffMetricsEnum metric, long value) {
+ setMetricValue(metric, value, null);
+ }
+
+ /*
+ Acronyms :-
+ 1.RCTSI :- Request count that succeeded in x retries
+ 2.MMA :- Min Max Average (This refers to the backoff or sleep time between 2
requests)
+ 3.s :- seconds
+ */
+ private void getRetryMetrics(StringBuilder metricBuilder) {
+ for (RetryValue retryCount : RETRY_LIST) {
+ long totalRequests = getMetricValue(TOTAL_REQUESTS, retryCount);
+ metricBuilder.append("$RCTSI$_").append(retryCount.getValue())
+
.append("R=").append(getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, retryCount));
+
+ if (totalRequests > 0) {
+ metricBuilder.append("$MMA$_").append(retryCount.getValue())
+ .append("R=").append(format(DOUBLE_PRECISION_FORMAT, (double)
getMetricValue(MIN_BACK_OFF, retryCount) / THOUSAND)).append("s")
Review Comment:
Here I was suggesting we can move the logic to format the value iside
getMetricValue() method itself.
We can have an overloaded `getMetricValue(metric, rertyValue, precision)`
which will first internally call the original `getMetricValue(metric,
retryValue)` and convert that based on precision.
This will simplify these redundant code lines.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBackoffMetrics.java:
##########
@@ -0,0 +1,275 @@
+/**
+ * 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.azurebfs.services;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum;
+import org.apache.hadoop.fs.azurebfs.enums.RetryValue;
+import org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum;
+import org.apache.hadoop.fs.azurebfs.statistics.AbstractAbfsStatisticsSource;
+import org.apache.hadoop.fs.statistics.impl.IOStatisticsStore;
+
+import static
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EMPTY_STRING;
+import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.THOUSAND;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.RETRY;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.COLON;
+import static
org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.DOUBLE_PRECISION_FORMAT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MIN_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_RETRY_COUNT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_BANDWIDTH_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_IOPS_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_NETWORK_FAILED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_OTHER_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_FAILED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED_WITHOUT_RETRYING;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.ONE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWO;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.THREE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FOUR;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIVE_FIFTEEN;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIFTEEN_TWENTY_FIVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWENTY_FIVE_AND_ABOVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
+import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_GAUGE;
+import static
org.apache.hadoop.fs.statistics.impl.IOStatisticsBinding.iostatisticsStore;
+import static org.apache.hadoop.util.StringUtils.format;
+import static org.apache.hadoop.util.StringUtils.formatPercent;
+
+/**
+ * This class is responsible for tracking and
+ * updating metrics related to backoff and
+ * retry operations in Azure Blob File System (ABFS).
+ */
+public class AbfsBackoffMetrics extends AbstractAbfsStatisticsSource {
+ private static final List<RetryValue> RETRY_LIST = Arrays.asList(ONE, TWO,
THREE, FOUR, FIVE_FIFTEEN, FIFTEEN_TWENTY_FIVE, TWENTY_FIVE_AND_ABOVE);
+
+ /**
+ * Constructor to initialize the IOStatisticsStore with counters and gauges.
+ */
+ public AbfsBackoffMetrics() {
+ IOStatisticsStore ioStatisticsStore = iostatisticsStore()
+ .withCounters(getMetricNames(TYPE_COUNTER))
+ .withGauges(getMetricNames(TYPE_GAUGE))
+ .build();
+ setIOStatistics(ioStatisticsStore);
+ }
+
+ /**
+ * Retrieves the metric names based on the statistic type.
+ *
+ * @param type the type of the statistic (counter or gauge)
+ * @return an array of metric names
+ */
+ private String[] getMetricNames(StatisticTypeEnum type) {
+ return Arrays.stream(AbfsBackoffMetricsEnum.values())
+ .filter(backoffMetricsEnum ->
backoffMetricsEnum.getStatisticType().equals(type))
+ .flatMap(backoffMetricsEnum ->
+ RETRY.equals(backoffMetricsEnum.getType())
+ ? RETRY_LIST.stream().map(retryCount ->
retryCount.getValue() + COLON + backoffMetricsEnum.getName())
+ : Stream.of(backoffMetricsEnum.getName())
+ ).toArray(String[]::new);
+ }
+
+ /**
+ * Constructs the metric name based on the metric and retry value.
+ *
+ * @param metric the metric enum
+ * @param retryValue the retry value
+ * @return the constructed metric name
+ */
+ private String getMetricName(AbfsBackoffMetricsEnum metric, RetryValue
retryValue) {
+ if (RETRY.equals(metric.getType())) {
+ return retryValue.getValue() + COLON + metric.getName();
+ }
+ return metric.getName();
+ }
+
+ /**
+ * Retrieves the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param retryValue the retry value
+ * @return the value of the metric
+ */
+ public long getMetricValue(AbfsBackoffMetricsEnum metric, RetryValue
retryValue) {
+ String metricName = getMetricName(metric, retryValue);
+ switch (metric.getStatisticType()) {
+ case TYPE_COUNTER:
+ return lookupCounterValue(metricName);
+ case TYPE_GAUGE:
+ return lookupGaugeValue(metricName);
+ default:
+ return 0;
+ }
+ }
+
+ /**
+ * Retrieves the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @return the value of the metric
+ */
+ public long getMetricValue(AbfsBackoffMetricsEnum metric) {
+ return getMetricValue(metric, null);
+ }
+
+ /**
+ * Increments the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param retryValue the retry value
+ */
+ public void incrementMetricValue(AbfsBackoffMetricsEnum metric, RetryValue
retryValue) {
+ String metricName = getMetricName(metric, retryValue);
+ switch (metric.getStatisticType()) {
+ case TYPE_COUNTER:
+ incCounterValue(metricName);
+ break;
+ case TYPE_GAUGE:
+ incGaugeValue(metricName);
+ break;
+ default:
+ // Do nothing
+ break;
+ }
+ }
+
+ /**
+ * Increments the value of a specific metric.
+ *
+ * @param metric the metric enum
+ */
+ public void incrementMetricValue(AbfsBackoffMetricsEnum metric) {
+ incrementMetricValue(metric, null);
+ }
+
+ /**
+ * Sets the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param value the new value of the metric
+ * @param retryValue the retry value
+ */
+ public void setMetricValue(AbfsBackoffMetricsEnum metric, long value,
RetryValue retryValue) {
+ String metricName = getMetricName(metric, retryValue);
+ switch (metric.getStatisticType()) {
+ case TYPE_COUNTER:
+ setCounterValue(metricName, value);
+ break;
+ case TYPE_GAUGE:
+ setGaugeValue(metricName, value);
+ break;
+ default:
+ // Do nothing
+ break;
+ }
+ }
+
+ /**
+ * Sets the value of a specific metric.
+ *
+ * @param metric the metric enum
+ * @param value the new value of the metric
+ */
+ public void setMetricValue(AbfsBackoffMetricsEnum metric, long value) {
+ setMetricValue(metric, value, null);
+ }
+
+ /*
+ Acronyms :-
+ 1.RCTSI :- Request count that succeeded in x retries
+ 2.MMA :- Min Max Average (This refers to the backoff or sleep time between 2
requests)
+ 3.s :- seconds
+ */
+ private void getRetryMetrics(StringBuilder metricBuilder) {
+ for (RetryValue retryCount : RETRY_LIST) {
+ long totalRequests = getMetricValue(TOTAL_REQUESTS, retryCount);
+ metricBuilder.append("$RCTSI$_").append(retryCount.getValue())
+
.append("R=").append(getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, retryCount));
+
+ if (totalRequests > 0) {
+ metricBuilder.append("$MMA$_").append(retryCount.getValue())
+ .append("R=").append(format(DOUBLE_PRECISION_FORMAT, (double)
getMetricValue(MIN_BACK_OFF, retryCount) / THOUSAND)).append("s")
+ .append(format(DOUBLE_PRECISION_FORMAT, (double)
getMetricValue(MAX_BACK_OFF, retryCount) / THOUSAND)).append("s")
Review Comment:
nit: move all `.append()` to new line for better readability. It will be
easier to see what all is appended individually.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBackoffMetrics.java:
##########
@@ -0,0 +1,275 @@
+/**
+ * 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.azurebfs.services;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum;
+import org.apache.hadoop.fs.azurebfs.enums.RetryValue;
+import org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum;
+import org.apache.hadoop.fs.azurebfs.statistics.AbstractAbfsStatisticsSource;
+import org.apache.hadoop.fs.statistics.impl.IOStatisticsStore;
+
+import static
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EMPTY_STRING;
+import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.THOUSAND;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.RETRY;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.COLON;
+import static
org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.DOUBLE_PRECISION_FORMAT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MIN_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_RETRY_COUNT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_BANDWIDTH_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_IOPS_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_NETWORK_FAILED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_OTHER_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_FAILED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED_WITHOUT_RETRYING;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.ONE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWO;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.THREE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FOUR;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIVE_FIFTEEN;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIFTEEN_TWENTY_FIVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWENTY_FIVE_AND_ABOVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
+import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_GAUGE;
+import static
org.apache.hadoop.fs.statistics.impl.IOStatisticsBinding.iostatisticsStore;
+import static org.apache.hadoop.util.StringUtils.format;
+import static org.apache.hadoop.util.StringUtils.formatPercent;
+
+/**
+ * This class is responsible for tracking and
+ * updating metrics related to backoff and
+ * retry operations in Azure Blob File System (ABFS).
+ */
+public class AbfsBackoffMetrics extends AbstractAbfsStatisticsSource {
+ private static final List<RetryValue> RETRY_LIST = Arrays.asList(ONE, TWO,
THREE, FOUR, FIVE_FIFTEEN, FIFTEEN_TWENTY_FIVE, TWENTY_FIVE_AND_ABOVE);
Review Comment:
nit: line length here and other places in this file
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java:
##########
@@ -24,6 +24,7 @@
import java.util.concurrent.atomic.AtomicLong;
Review Comment:
I think it will be good to move this class and other statistics related
classes(like AbfsStatistics.java etc) inside the new `statistics` package that
you have introduced.
Feel free to take it up as a separate work item, out of scope of this PR.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/enums/FileType.java:
##########
@@ -0,0 +1,30 @@
+/**
+ * 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.azurebfs.enums;
+
Review Comment:
Javadoc for class and where/how it is used
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/enums/AbfsReadFooterMetricsEnum.java:
##########
@@ -0,0 +1,93 @@
+/**
+ * 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.azurebfs.enums;
+
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.FILE;
+import static
org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
+import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_MEAN;
+
+/**
+ * Enum representing various ABFS read footer metrics.
+ */
+public enum AbfsReadFooterMetricsEnum {
+ TOTAL_FILES("totalFiles", "Total files read", FILE, TYPE_COUNTER),
+ AVG_FILE_LENGTH("avgFileLength", "Average File length", FILE, TYPE_MEAN),
+ AVG_SIZE_READ_BY_FIRST_READ("avgSizeReadByFirstRead", "Average Size read
by first read", FILE, TYPE_MEAN),
+
AVG_OFFSET_DIFF_BETWEEN_FIRST_AND_SECOND_READ("avgOffsetDiffBetweenFirstAndSecondRead",
+ "Average Offset difference between first and second read", FILE,
TYPE_MEAN),
+ AVG_READ_LEN_REQUESTED("avgReadLenRequested", "Average Read length
requested", FILE, TYPE_MEAN),
+ AVG_FIRST_OFFSET_DIFF("avgFirstOffsetDiff", "Average First offset
difference", FILE, TYPE_MEAN),
+ AVG_SECOND_OFFSET_DIFF("avgSecondOffsetDiff", "Average Second offset
difference", FILE, TYPE_MEAN);
+
+ private final String name;
+ private final String description;
+ private final String type;
+ private final StatisticTypeEnum statisticType;
+
+ /**
+ * Constructor for AbfsReadFooterMetricsEnum.
+ *
+ * @param name the name of the metric
+ * @param description the description of the metric
+ * @param type the type of the metric (FILE)
+ * @param statisticType the statistic type of the metric (counter or gauge)
+ */
+ AbfsReadFooterMetricsEnum(String name, String description, String type,
StatisticTypeEnum statisticType) {
Review Comment:
nit: line length
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBackoffMetrics.java:
##########
@@ -0,0 +1,275 @@
+/**
+ * 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.azurebfs.services;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum;
+import org.apache.hadoop.fs.azurebfs.enums.RetryValue;
+import org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum;
+import org.apache.hadoop.fs.azurebfs.statistics.AbstractAbfsStatisticsSource;
+import org.apache.hadoop.fs.statistics.impl.IOStatisticsStore;
+
+import static
org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EMPTY_STRING;
+import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.THOUSAND;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.RETRY;
+import static org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.COLON;
+import static
org.apache.hadoop.fs.azurebfs.constants.MetricsConstants.DOUBLE_PRECISION_FORMAT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MIN_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.MAX_RETRY_COUNT;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_BANDWIDTH_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_IOPS_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_NETWORK_FAILED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_OTHER_THROTTLED_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_FAILED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED_WITHOUT_RETRYING;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_BACK_OFF;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_REQUESTS;
+import static
org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.ONE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWO;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.THREE;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FOUR;
+import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIVE_FIFTEEN;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.FIFTEEN_TWENTY_FIVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWENTY_FIVE_AND_ABOVE;
+import static
org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
+import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_GAUGE;
+import static
org.apache.hadoop.fs.statistics.impl.IOStatisticsBinding.iostatisticsStore;
+import static org.apache.hadoop.util.StringUtils.format;
+import static org.apache.hadoop.util.StringUtils.formatPercent;
+
+/**
+ * This class is responsible for tracking and
+ * updating metrics related to backoff and
+ * retry operations in Azure Blob File System (ABFS).
+ */
+public class AbfsBackoffMetrics extends AbstractAbfsStatisticsSource {
Review Comment:
This class can also be moved to statistics package.
Again, feel free to take it as a separate WI
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/statistics/package-info.java:
##########
@@ -0,0 +1,23 @@
+/*
Review Comment:
Makes sense, we can take that up as a separate Work item.
But its good to keep package structure clean.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBackoffMetrics.java:
##########
@@ -202,51 +204,68 @@ public void setMetricValue(AbfsBackoffMetricsEnum metric,
long value) {
1.RCTSI :- Request count that succeeded in x retries
2.MMA :- Min Max Average (This refers to the backoff or sleep time between 2
requests)
3.s :- seconds
- 4.BWT :- Number of Bandwidth throttled requests
- 5.IT :- Number of IOPS throttled requests
- 6.OT :- Number of Other throttled requests
- 7.NFR :- Number of requests which failed due to network errors
- 8.%RT :- Percentage of requests that are throttled
- 9.TRNR :- Total number of requests which succeeded without retrying
- 10.TRF :- Total number of requests which failed
- 11.TR :- Total number of requests which were made
- 12.MRC :- Max retry count across all requests
*/
- @Override
- public String toString() {
- if (getMetricValue(TOTAL_NUMBER_OF_REQUESTS) == 0) {
- return "";
+ private void getRetryMetrics(StringBuilder metricBuilder) {
+ for (RetryValue retryCount : RETRY_LIST) {
+ long totalRequests = getMetricValue(TOTAL_REQUESTS, retryCount);
+ metricBuilder.append("$RCTSI$_").append(retryCount.getValue())
Review Comment:
+1
All these constants are used specifically for Metric work. They qualify to
be added into `MetricConstants` class. It make sesne to retain that class and
add all these constants there itself.
> [ABFS] Implement Backoff and Read Footer metrics using IOStatistics Class
> -------------------------------------------------------------------------
>
> Key: HADOOP-19311
> URL: https://issues.apache.org/jira/browse/HADOOP-19311
> Project: Hadoop Common
> Issue Type: Task
> Components: fs/azure
> Affects Versions: 3.5.0, 3.4.1
> Reporter: Manish Bhatt
> Assignee: Manish Bhatt
> Priority: Major
>
> Current Flow: We have implemented metrics collection in ABFS flow. We have
> created a custom AbfsBackoffMetrics and AbfsReadFooterMetrics class which
> stores all the metrics on the file system level. Our objective is to move
> away from the custom class implementation and use IOStatisticsStore to store
> the metrics which is present in hadoop-common.
> Changes Made: This PR contains the changes related to storing metrics related
> to above mentioned classes in IOStatisticStore which is present in
> hadoop-common. AbstractAbfsStatisticsSource abstract class is created which
> is implementing IOStatisticsSource interface. This will store IOStatistics of
> the child metrics class.
> Both AbfsBackoffMetrics and AbfsReadFooterMetrics is inheriting
> AbstractAbfsStatisticsSource and store the respective metrics in
> IOStatisticsStore.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
