From a53b8fe46288fdbacea025af63cb7d50a7aecbbc Mon Sep 17 00:00:00 2001 From: Zamil Majdy Date: Thu, 4 Nov 2021 18:03:45 +0100 Subject: [PATCH 1/3] HADOOP-17990. Fix failing concurrent FS.initialize commands when fs.azure.createRemoteFileSystemDuringInitialization is enabled. --- .../fs/azurebfs/AzureBlobFileSystem.java | 32 ++++++++++---- ...ITestAzureBlobFileSystemInitAndCreate.java | 42 +++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java index 91274289f54156..5e05745fb047b6 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java @@ -150,13 +150,7 @@ public void initialize(URI uri, Configuration configuration) if (abfsConfiguration.getCreateRemoteFileSystemDuringInitialization()) { TracingContext tracingContext = new TracingContext(clientCorrelationId, fileSystemId, FSOperationType.CREATE_FILESYSTEM, tracingHeaderFormat, listener); - if (this.tryGetFileStatus(new Path(AbfsHttpConstants.ROOT_PATH), tracingContext) == null) { - try { - this.createFileSystem(tracingContext); - } catch (AzureBlobFileSystemException ex) { - checkException(null, ex, AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS); - } - } + this.createFileSystemIfNotExist(tracingContext); } LOG.trace("Initiate check for delegation token manager"); @@ -1411,6 +1405,30 @@ AzureBlobFileSystemStore getAbfsStore() { return abfsStore; } + @VisibleForTesting + void setAbfsStore(AzureBlobFileSystemStore abfsStore) { + this.abfsStore = abfsStore; + } + + @VisibleForTesting + void createFileSystemIfNotExist(TracingContext tracingContext) throws IOException { + if (this.tryGetFileStatus(new Path(AbfsHttpConstants.ROOT_PATH), tracingContext) == null) { + try { + this.createFileSystem(tracingContext); + } catch (IOException ex) { + if (ex instanceof AzureBlobFileSystemException) { + checkException(null, (AzureBlobFileSystemException) ex, + AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS); + } else if (ex.getCause() != null && ex.getCause() instanceof AzureBlobFileSystemException) { + checkException(null, (AzureBlobFileSystemException) ex.getCause(), + AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS); + } else { + throw ex; + } + } + } + } + @VisibleForTesting AbfsClient getAbfsClient() { return abfsStore.getClient(); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java index 5c4b87b0d2f4a8..d8df7bf832c0b7 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java @@ -19,9 +19,16 @@ package org.apache.hadoop.fs.azurebfs; import java.io.FileNotFoundException; +import java.io.IOException; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException; +import org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode; +import org.apache.hadoop.fs.azurebfs.utils.TracingContext; import org.junit.Test; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; @@ -49,4 +56,39 @@ public void ensureFilesystemWillNotBeCreatedIfCreationConfigIsNotSet() throws Ex final AzureBlobFileSystem fs = this.createFileSystem(); FileStatus[] fileStatuses = fs.listStatus(new Path("/")); } + + @Test + public void ensureFilesystemWillBeCreatedIfCreationConfigIsSet() throws Exception { + final AzureBlobFileSystem fs = createFileSystem(); + Configuration config = getRawConfiguration(); + fs.initialize(FileSystem.getDefaultUri(config), config); + + // Make sure createFileSystemIfNotExists is working as intended. + final MockAzureBlobFileSystemStore store = new MockAzureBlobFileSystemStore(config); + fs.setAbfsStore(store); + fs.createFileSystemIfNotExist(getTestTracingContext(fs, true)); + assert(store.isCreateFileSystemCalled); + } + + /** + * Mock AzureBlobFileSystemStore to simulate container already exists + * exception when calling createFileSystem command. + */ + static class MockAzureBlobFileSystemStore extends AzureBlobFileSystemStore { + boolean isCreateFileSystemCalled = false; + + public MockAzureBlobFileSystemStore(Configuration config) throws IOException { + super(FileSystem.getDefaultUri(config), true, config, null); + } + + @Override + public void createFilesystem(TracingContext tracingContext) throws AzureBlobFileSystemException { + isCreateFileSystemCalled = true; + // Make sure createFileSystemIfNotExists works when the filesystem/container already exists. + throw new AbfsRestOperationException( + AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS.getStatusCode(), + AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS.getErrorCode(), + "This container is already exists", null); + } + } } From 2fa8e9f4c9ad9178111ef44ea39cac77621714c3 Mon Sep 17 00:00:00 2001 From: Zamil Majdy Date: Mon, 8 Nov 2021 14:53:04 +0100 Subject: [PATCH 2/3] Address comments --- .../fs/azurebfs/AzureBlobFileSystem.java | 8 ++++---- .../ITestAzureBlobFileSystemInitAndCreate.java | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java index 5e05745fb047b6..c24eee20534b1e 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java @@ -1415,11 +1415,11 @@ void createFileSystemIfNotExist(TracingContext tracingContext) throws IOExceptio if (this.tryGetFileStatus(new Path(AbfsHttpConstants.ROOT_PATH), tracingContext) == null) { try { this.createFileSystem(tracingContext); + } catch (AzureBlobFileSystemException ex) { + checkException(null, (AzureBlobFileSystemException) ex, + AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS); } catch (IOException ex) { - if (ex instanceof AzureBlobFileSystemException) { - checkException(null, (AzureBlobFileSystemException) ex, - AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS); - } else if (ex.getCause() != null && ex.getCause() instanceof AzureBlobFileSystemException) { + if (ex.getCause() instanceof AzureBlobFileSystemException) { checkException(null, (AzureBlobFileSystemException) ex.getCause(), AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS); } else { diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java index d8df7bf832c0b7..67db4c13200797 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java @@ -59,15 +59,17 @@ public void ensureFilesystemWillNotBeCreatedIfCreationConfigIsNotSet() throws Ex @Test public void ensureFilesystemWillBeCreatedIfCreationConfigIsSet() throws Exception { - final AzureBlobFileSystem fs = createFileSystem(); - Configuration config = getRawConfiguration(); - fs.initialize(FileSystem.getDefaultUri(config), config); + try (AzureBlobFileSystem fs = createFileSystem()) { + Configuration config = getRawConfiguration(); + fs.initialize(FileSystem.getDefaultUri(config), config); - // Make sure createFileSystemIfNotExists is working as intended. - final MockAzureBlobFileSystemStore store = new MockAzureBlobFileSystemStore(config); - fs.setAbfsStore(store); - fs.createFileSystemIfNotExist(getTestTracingContext(fs, true)); - assert(store.isCreateFileSystemCalled); + // Make sure createFileSystemIfNotExists is working as intended. + final MockAzureBlobFileSystemStore store = new MockAzureBlobFileSystemStore(config); + fs.setAbfsStore(store); + fs.createFileSystemIfNotExist(getTestTracingContext(fs, true)); + assertTrue("Expected AzureBlobFileSystemStore.createFilesystem to be called", + store.isCreateFileSystemCalled); + } } /** From 09264770f5adfcba272d4664e11f6fd1da178974 Mon Sep 17 00:00:00 2001 From: Zamil Majdy Date: Wed, 10 Nov 2021 09:35:54 +0100 Subject: [PATCH 3/3] Fix style error --- .../fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java index 67db4c13200797..6194710a24f9b6 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java @@ -77,9 +77,9 @@ public void ensureFilesystemWillBeCreatedIfCreationConfigIsSet() throws Exceptio * exception when calling createFileSystem command. */ static class MockAzureBlobFileSystemStore extends AzureBlobFileSystemStore { - boolean isCreateFileSystemCalled = false; + private boolean isCreateFileSystemCalled = false; - public MockAzureBlobFileSystemStore(Configuration config) throws IOException { + MockAzureBlobFileSystemStore(Configuration config) throws IOException { super(FileSystem.getDefaultUri(config), true, config, null); }