From 27f836335432b6205f85f59bc85c8e07d93fe810 Mon Sep 17 00:00:00 2001 From: Anmol Asrani Date: Mon, 27 May 2024 22:58:32 -0700 Subject: [PATCH 1/4] Fix test failures due to missing config --- .../fs/azurebfs/services/AbfsClient.java | 30 +++++++++++-------- .../azurebfs/ITestAbfsReadFooterMetrics.java | 19 ++++++++++++ .../services/TestAbfsRestOperation.java | 19 ++++++++++++ 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index f4ff181357960..87b8406bb2981 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -202,21 +202,25 @@ private AbfsClient(final URL baseUrl, this.isMetricCollectionStopped = new AtomicBoolean(false); this.metricAnalysisPeriod = abfsConfiguration.getMetricAnalysisTimeout(); this.metricIdlePeriod = abfsConfiguration.getMetricIdleTimeout(); - if (!metricFormat.toString().equals("")) { - isMetricCollectionEnabled = true; - abfsCounters.initializeMetrics(metricFormat); + if (!metricFormat.toString().equals(EMPTY_STRING)) { String metricAccountName = abfsConfiguration.getMetricAccount(); - int dotIndex = metricAccountName.indexOf(AbfsHttpConstants.DOT); - if (dotIndex <= 0) { - throw new InvalidUriException( - metricAccountName + " - account name is not fully qualified."); - } String metricAccountKey = abfsConfiguration.getMetricAccountKey(); - try { - metricSharedkeyCredentials = new SharedKeyCredentials(metricAccountName.substring(0, dotIndex), - metricAccountKey); - } catch (IllegalArgumentException e) { - throw new IOException("Exception while initializing metric credentials " + e); + if (!metricAccountName.equals(EMPTY_STRING) && !metricAccountKey.equals(EMPTY_STRING)) { + isMetricCollectionEnabled = true; + abfsCounters.initializeMetrics(metricFormat); + int dotIndex = metricAccountName.indexOf(AbfsHttpConstants.DOT); + if (dotIndex <= 0) { + throw new InvalidUriException( + metricAccountName + " - account name is not fully qualified."); + } + try { + metricSharedkeyCredentials = new SharedKeyCredentials( + metricAccountName.substring(0, dotIndex), + metricAccountKey); + } catch (IllegalArgumentException e) { + throw new IOException( + "Exception while initializing metric credentials " + e); + } } } this.timer = new Timer( diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsReadFooterMetrics.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsReadFooterMetrics.java index 0071b90771c49..90d769b56f4b9 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsReadFooterMetrics.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsReadFooterMetrics.java @@ -21,7 +21,10 @@ import static org.apache.hadoop.fs.CommonConfigurationKeys.IOSTATISTICS_LOGGING_LEVEL_INFO; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_READ_BUFFER_SIZE; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_WRITE_BUFFER_SIZE; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_ACCOUNT_KEY; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_ACCOUNT_NAME; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_FORMAT; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_URI; import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.MIN_BUFFER_SIZE; import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.ONE_KB; import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.ONE_MB; @@ -30,6 +33,8 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.azurebfs.utils.MetricFormat; + +import org.junit.Assume; import org.junit.Test; import java.io.IOException; @@ -47,6 +52,20 @@ public class ITestAbfsReadFooterMetrics extends AbstractAbfsScaleTest { public ITestAbfsReadFooterMetrics() throws Exception { + checkPrerequisites(); + } + + private void checkPrerequisites(){ + checkIfConfigIsSet(FS_AZURE_METRIC_ACCOUNT_NAME); + checkIfConfigIsSet(FS_AZURE_METRIC_ACCOUNT_KEY); + checkIfConfigIsSet(FS_AZURE_METRIC_URI); + } + + private void checkIfConfigIsSet(String configKey){ + AbfsConfiguration conf = getConfiguration(); + String value = conf.get(configKey); + Assume.assumeTrue(configKey + " config is mandatory for the test to run", + value != null && value.trim().length() > 1); } private static final String TEST_PATH = "/testfile"; diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java index 683528b9c54d1..8ab8d07d7287d 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java @@ -22,9 +22,14 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.utils.MetricFormat; + +import org.junit.Assume; import org.junit.Test; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_DELETE; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_ACCOUNT_KEY; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_ACCOUNT_NAME; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_FORMAT; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_URI; import static org.apache.hadoop.fs.azurebfs.services.AbfsRestOperationType.DeletePath; import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem; import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest; @@ -39,6 +44,19 @@ public class TestAbfsRestOperation extends public TestAbfsRestOperation() throws Exception { } + private void checkPrerequisites() throws Exception { + checkIfConfigIsSet(FS_AZURE_METRIC_ACCOUNT_NAME); + checkIfConfigIsSet(FS_AZURE_METRIC_ACCOUNT_KEY); + checkIfConfigIsSet(FS_AZURE_METRIC_URI); + } + + private void checkIfConfigIsSet(String configKey){ + AbfsConfiguration conf = getConfiguration(); + String value = conf.get(configKey); + Assume.assumeTrue(configKey + " config is mandatory for the test to run", + value != null && value.trim().length() > 1); + } + /** * Test for backoff retry metrics. * @@ -49,6 +67,7 @@ public TestAbfsRestOperation() throws Exception { */ @Test public void testBackoffRetryMetrics() throws Exception { + checkPrerequisites(); // Create an AzureBlobFileSystem instance. final Configuration configuration = getRawConfiguration(); configuration.set(FS_AZURE_METRIC_FORMAT, String.valueOf(MetricFormat.INTERNAL_BACKOFF_METRIC_FORMAT)); From b11c271e6c351b8dfc9e95cc04cd9985a651907f Mon Sep 17 00:00:00 2001 From: Anmol Asrani Date: Mon, 10 Jun 2024 02:29:31 -0700 Subject: [PATCH 2/4] review comments --- .../org/apache/hadoop/fs/azurebfs/services/AbfsClient.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 87b8406bb2981..319bd3e21b487 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -41,6 +41,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.classification.VisibleForTesting; import org.apache.hadoop.fs.azurebfs.constants.FSOperationType; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsInvalidChecksumException; @@ -205,7 +206,7 @@ private AbfsClient(final URL baseUrl, if (!metricFormat.toString().equals(EMPTY_STRING)) { String metricAccountName = abfsConfiguration.getMetricAccount(); String metricAccountKey = abfsConfiguration.getMetricAccountKey(); - if (!metricAccountName.equals(EMPTY_STRING) && !metricAccountKey.equals(EMPTY_STRING)) { + if (StringUtils.isNotEmpty(metricAccountName) && StringUtils.isNotEmpty(metricAccountKey)) { isMetricCollectionEnabled = true; abfsCounters.initializeMetrics(metricFormat); int dotIndex = metricAccountName.indexOf(AbfsHttpConstants.DOT); @@ -218,8 +219,7 @@ private AbfsClient(final URL baseUrl, metricAccountName.substring(0, dotIndex), metricAccountKey); } catch (IllegalArgumentException e) { - throw new IOException( - "Exception while initializing metric credentials " + e); + throw new IOException("Exception while initializing metric credentials ", e); } } } From cdbc43fc551c296c4ca7a2562702322a89edc9a6 Mon Sep 17 00:00:00 2001 From: Anmol Asrani Date: Mon, 10 Jun 2024 02:36:08 -0700 Subject: [PATCH 3/4] metric fix --- .../java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 319bd3e21b487..d09f62def9a44 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -203,7 +203,7 @@ private AbfsClient(final URL baseUrl, this.isMetricCollectionStopped = new AtomicBoolean(false); this.metricAnalysisPeriod = abfsConfiguration.getMetricAnalysisTimeout(); this.metricIdlePeriod = abfsConfiguration.getMetricIdleTimeout(); - if (!metricFormat.toString().equals(EMPTY_STRING)) { + if (StringUtils.isNotEmpty(metricFormat.toString())) { String metricAccountName = abfsConfiguration.getMetricAccount(); String metricAccountKey = abfsConfiguration.getMetricAccountKey(); if (StringUtils.isNotEmpty(metricAccountName) && StringUtils.isNotEmpty(metricAccountKey)) { From fcf6e0db464167087b215fe96cc88d7c7aca7d25 Mon Sep 17 00:00:00 2001 From: Anmol Asrani Date: Tue, 19 Nov 2024 22:48:42 -0800 Subject: [PATCH 4/4] remove extra lines --- .../hadoop/fs/azurebfs/services/TestAbfsRestOperation.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java index b162c4146129a..e5fcf9e71ed4f 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRestOperation.java @@ -22,17 +22,13 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.utils.MetricFormat; - import org.junit.Test; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_DELETE; - import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_ACCOUNT_KEY; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_ACCOUNT_NAME; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_FORMAT; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_METRIC_URI; - import static org.apache.hadoop.fs.azurebfs.services.AbfsRestOperationType.DeletePath; - import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem; import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest; import java.util.ArrayList;