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. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
