From 0bfe00728dc19c14378d0a2aa58842330a1b5f4a Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Thu, 13 Feb 2025 11:00:06 -0800 Subject: [PATCH 1/6] Rename Testcase for Blob endpoint --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 2 +- .../hadoop/fs/azurebfs/AbfsCountersImpl.java | 4 +- .../hadoop/fs/azurebfs/AbfsStatistic.java | 4 +- .../fs/azurebfs/services/AbfsBlobClient.java | 14 +- .../fs/azurebfs/services/AbfsClient.java | 34 + .../azurebfs/services/BlobDeleteHandler.java | 1 + .../azurebfs/services/BlobRenameHandler.java | 3 + .../fs/azurebfs/services/RenameAtomicity.java | 4 + .../ITestAzureBlobFileSystemRename.java | 841 ++++++++++++++++++ 9 files changed, 897 insertions(+), 10 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index e0cb36201065d..6d2212a17b2c4 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -415,7 +415,7 @@ public class AbfsConfiguration{ private int producerQueueMaxSize; @IntegerConfigurationValidatorAnnotation(ConfigurationKey = - FS_AZURE_CONSUMER_MAX_LAG, DefaultValue = DEFAULT_FS_AZURE_CONSUMER_MAX_LAG) + FS_AZURE_CONSUMER_MAX_LAG, DefaultValue = DEFAULT_FS_AZURE_CONSUMER_MAX_LAG) private int listingMaxConsumptionLag; @IntegerConfigurationValidatorAnnotation(ConfigurationKey = diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java index fdcbc2275ff48..9c5b13ce2472e 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java @@ -54,6 +54,7 @@ import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_CREATED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.ERROR_IGNORED; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_COPIED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_CREATED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.GET_RESPONSES; @@ -134,7 +135,8 @@ public class AbfsCountersImpl implements AbfsCounters { SERVER_UNAVAILABLE, RENAME_RECOVERY, METADATA_INCOMPLETE_RENAME_FAILURES, - RENAME_PATH_ATTEMPTS + RENAME_PATH_ATTEMPTS, + FILES_COPIED }; private static final AbfsStatistic[] DURATION_TRACKER_LIST = { diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java index 3a77e82ffb4fb..bf2a4c8679e9c 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java @@ -109,7 +109,9 @@ public enum AbfsStatistic { "Number of times rename operation failed due to metadata being " + "incomplete"), RENAME_PATH_ATTEMPTS("rename_path_attempts", - "Number of times we attempt to rename a path internally"); + "Number of times we attempt to rename a path internally"), + FILES_COPIED("files_copied", + "Total number of files copied from the object store."); private String statName; private String statDescription; diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java index b54ce1a4dac7e..a9c06526f9a9d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java @@ -424,7 +424,7 @@ private void fixAtomicEntriesInListResults(final AbfsRestOperation op, } List filteredEntries = new ArrayList<>(); for (BlobListResultEntrySchema entry : listResultSchema.paths()) { - if (!takeListPathAtomicRenameKeyAction(entry.path(), + if (!takeListPathAtomicRenameKeyAction(entry.path(), entry.isDirectory(), entry.contentLength().intValue(), tracingContext)) { filteredEntries.add(entry); } @@ -444,6 +444,7 @@ public void createNonRecursivePreCheck(Path parentPath, } getPathStatus(parentPath.toUri().getPath(), false, tracingContext, null); + incrementAbfsGetPathStatus(); } catch (AbfsRestOperationException ex) { if (ex.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) { throw new FileNotFoundException("Cannot create file " @@ -451,8 +452,6 @@ public void createNonRecursivePreCheck(Path parentPath, + " because parent folder does not exist."); } throw ex; - } finally { - getAbfsCounters().incrementCounter(CALL_GET_FILE_STATUS, 1); } } @@ -510,6 +509,7 @@ protected AbfsRestOperation createMarkerAtPath(final String path, final String eTag, final ContextEncryptionAdapter contextEncryptionAdapter, final TracingContext tracingContext) throws AzureBlobFileSystemException { + incrementAbfsCreateFile(); return createPathRestOp(path, false, false, false, eTag, contextEncryptionAdapter, tracingContext); } @@ -807,7 +807,6 @@ public AbfsClientRenameResult renamePath(final String source, BlobRenameHandler blobRenameHandler = getBlobRenameHandler(source, destination, sourceEtag, isAtomicRenameKey(source), tracingContext ); - incrementAbfsRenamePath(); if (blobRenameHandler.execute()) { final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); @@ -1751,6 +1750,7 @@ public void takeGetPathStatusAtomicRenameKeyAction(final Path path, pendingJsonFileStatus = getPathStatus( pendingJsonPath.toUri().getPath(), tracingContext, null, false); + incrementAbfsGetPathStatus(); if (checkIsDir(pendingJsonFileStatus.getResult())) { return; } @@ -1805,11 +1805,11 @@ public void takeGetPathStatusAtomicRenameKeyAction(final Path path, * @throws AzureBlobFileSystemException server error */ private boolean takeListPathAtomicRenameKeyAction(final Path path, - final int renamePendingJsonLen, + final boolean isDirectory, final int renamePendingJsonLen, final TracingContext tracingContext) throws AzureBlobFileSystemException { if (path == null || path.isRoot() || !isAtomicRenameKey( - path.toUri().getPath()) || !path.toUri() + path.toUri().getPath()) || isDirectory || !path.toUri() .getPath() .endsWith(RenameAtomicity.SUFFIX)) { return false; @@ -1837,7 +1837,7 @@ private boolean takeListPathAtomicRenameKeyAction(final Path path, } @VisibleForTesting - RenameAtomicity getRedoRenameAtomicity(final Path renamePendingJsonPath, + public RenameAtomicity getRedoRenameAtomicity(final Path renamePendingJsonPath, int fileLen, final TracingContext tracingContext) { return new RenameAtomicity(renamePendingJsonPath, 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 b6aca80768249..754487b62cd7c 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 @@ -91,6 +91,12 @@ import org.apache.hadoop.util.concurrent.HadoopExecutors; import static org.apache.commons.lang3.StringUtils.isNotEmpty; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_CREATED; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_DELETED; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_COPIED; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_CREATED; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.RENAME_PATH_ATTEMPTS; import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APN_VERSION; @@ -693,6 +699,34 @@ protected void incrementAbfsRenamePath() { abfsCounters.incrementCounter(RENAME_PATH_ATTEMPTS, 1); } + /** + * Increments AbfsCounters for get path status by 1. + */ + protected void incrementAbfsGetPathStatus() { + abfsCounters.incrementCounter(CALL_GET_FILE_STATUS, 1); + } + + /** + * Increments AbfsCounters for Delete File by 1. + */ + protected void incrementAbfsDeleteFile() { + abfsCounters.incrementCounter(FILES_DELETED, 1); + } + + /** + * Increments AbfsCounters for Create File by 1. + */ + protected void incrementAbfsCreateFile() { + abfsCounters.incrementCounter(FILES_CREATED, 1); + } + + /** + * Increments AbfsCounters for Copy Files by 1. + */ + protected void incrementAbfsCopyFile() { + abfsCounters.incrementCounter(FILES_COPIED, 1); + } + /** * Check if the rename request failure is post a retry and if earlier rename * request might have succeeded at back-end. diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java index d93bafb676cb3..4963b288d3438 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java @@ -90,6 +90,7 @@ int getMaxConsumptionParallelism() { private boolean deleteInternal(final Path path) throws AzureBlobFileSystemException { getAbfsClient().deleteBlobPath(path, null, tracingContext); + getAbfsClient().incrementAbfsDeleteFile(); deleteCount.incrementAndGet(); return true; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java index 695c0694cf533..b9583e1fb41cf 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java @@ -121,6 +121,7 @@ int getMaxConsumptionParallelism() { * @throws AzureBlobFileSystemException if server call fails */ public boolean execute() throws AzureBlobFileSystemException { + getAbfsClient().incrementAbfsRenamePath(); PathInformation pathInformation = getPathInformation(src, tracingContext); boolean result = false; if (preCheck(src, dst, pathInformation)) { @@ -439,7 +440,9 @@ private boolean renameInternal(final Path path, boolean operated = false; try { copyPath(path, destinationPathForBlobPartOfRenameSrcDir, leaseId); + getAbfsClient().incrementAbfsCopyFile(); getAbfsClient().deleteBlobPath(path, leaseId, tracingContext); + getAbfsClient().incrementAbfsDeleteFile(); operated = true; } finally { if (abfsLease != null) { diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java index f8dab188f37eb..72e1c0d379a62 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java @@ -267,6 +267,8 @@ public int preRename() throws AzureBlobFileSystemException { } } throw e; + } finally { + abfsClient.incrementAbfsCreateFile(); } } @@ -310,6 +312,8 @@ private void deleteRenamePendingJson() throws AzureBlobFileSystemException { return; } throw e; + } finally { + abfsClient.incrementAbfsDeleteFile(); } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index abd45eae0e087..63593ce94d79f 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -23,8 +23,10 @@ import java.nio.charset.StandardCharsets; import java.nio.file.AccessDeniedException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -41,10 +43,12 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.PathIOException; import org.apache.hadoop.fs.azurebfs.constants.FSOperationType; import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; 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.security.ContextEncryptionAdapter; import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient; import org.apache.hadoop.fs.azurebfs.services.AbfsClient; @@ -57,6 +61,7 @@ import org.apache.hadoop.fs.azurebfs.services.RenameAtomicity; import org.apache.hadoop.fs.azurebfs.services.RenameAtomicityTestUtils; import org.apache.hadoop.fs.azurebfs.utils.TracingContext; +import org.apache.hadoop.fs.azurebfs.utils.TracingHeaderFormat; import org.apache.hadoop.fs.azurebfs.utils.TracingHeaderValidator; import org.apache.hadoop.fs.statistics.IOStatisticAssertions; import org.apache.hadoop.fs.statistics.IOStatistics; @@ -66,14 +71,23 @@ import static java.net.HttpURLConnection.HTTP_CONFLICT; import static java.net.HttpURLConnection.HTTP_FORBIDDEN; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_OK; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.RENAME_PATH_ATTEMPTS; +import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.COPY_STATUS_ABORTED; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.COPY_STATUS_FAILED; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.COPY_STATUS_PENDING; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ROOT_PATH; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ATOMIC_RENAME_KEY; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_BLOB_DIR_RENAME_MAX_THREAD; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_CONSUMER_MAX_LAG; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_LEASE_THREADS; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_PRODUCER_QUEUE_MAX_SIZE; +import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.BLOB_ALREADY_EXISTS; +import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.BLOB_PATH_NOT_FOUND; import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.COPY_BLOB_ABORTED; import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.COPY_BLOB_FAILED; +import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.PATH_NOT_FOUND; import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.SOURCE_PATH_NOT_FOUND; import static org.apache.hadoop.fs.azurebfs.services.RenameAtomicity.SUFFIX; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsFile; @@ -95,6 +109,12 @@ public class ITestAzureBlobFileSystemRename extends private static final int BLOB_COUNT = 11; + private static final int TOTAL_FILES = 25; + + private static final int TOTAL_THREADS_IN_POOL = 5; + + private static final int FAILED_CALL = 15; + public ITestAzureBlobFileSystemRename() throws Exception { super(); } @@ -1641,4 +1661,825 @@ public void testRenameSrcDirDeleteEmitDeletionCountInClientRequestId() Mockito.any(TracingContext.class)); fs.rename(new Path(dirPathStr), new Path("/dst/")); } + + /** + * Helper method to configure the AzureBlobFileSystem and rename directories. + * + * @param currentFs The current AzureBlobFileSystem to use for renaming. + * @param producerQueueSize Maximum size of the producer queue. + * @param consumerMaxLag Maximum lag allowed for the consumer. + * @param maxThread Maximum threads for the rename operation. + * @param src The source path of the directory to rename. + * @param dst The destination path of the renamed directory. + * @throws IOException If an I/O error occurs during the operation. + */ + private void renameDir(AzureBlobFileSystem currentFs, String producerQueueSize, + String consumerMaxLag, String maxThread, Path src, Path dst) + throws IOException { + Configuration config = createConfig(producerQueueSize, consumerMaxLag, maxThread); + try (final AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(currentFs.getUri(), config)) { + fs.rename(src, dst); + validateRename(fs, src, dst, false, true, false); + } + } + + /** + * Helper method to create the configuration for the AzureBlobFileSystem. + * + * @param producerQueueSize Maximum size of the producer queue. + * @param consumerMaxLag Maximum lag allowed for the consumer. + * @param maxThread Maximum threads for the rename operation. + * @return The configuration object. + */ + private Configuration createConfig(String producerQueueSize, String consumerMaxLag, String maxThread) { + Configuration config = new Configuration(this.getRawConfiguration()); + config.set(FS_AZURE_PRODUCER_QUEUE_MAX_SIZE, producerQueueSize); + config.set(FS_AZURE_CONSUMER_MAX_LAG, consumerMaxLag); + config.set(FS_AZURE_BLOB_DIR_RENAME_MAX_THREAD, maxThread); + return config; + } + + /** + * Helper method to validate that the rename was successful and that the destination exists. + * + * @param fs The AzureBlobFileSystem instance to check the existence on. + * @param dst The destination path. + * @param src The source path. + * @throws IOException If an I/O error occurs during the validation. + */ + private void validateRename(AzureBlobFileSystem fs, Path src, Path dst, + boolean isSrcExist, boolean isDstExist, boolean isJsonExist) + throws IOException { + Assertions.assertThat(fs.exists(dst)) + .describedAs("Renamed Destination directory should exist.") + .isEqualTo(isDstExist); + Assertions.assertThat(fs.exists(new Path(src.getParent(), src.getName() + SUFFIX))) + .describedAs("Renamed Pending Json file should exist.") + .isEqualTo(isJsonExist); + Assertions.assertThat(fs.exists(src)) + .describedAs("Renamed Destination directory should exist.") + .isEqualTo(isSrcExist); + } + + /** + * Test the renaming of a directory with different parallelism configurations. + */ + @Test + public void testRenameDirWithDifferentParallelism() throws Exception { + try (final AzureBlobFileSystem currentFs = getFileSystem()) { + assumeBlobServiceType(); + Path src = new Path("/hbase/A1/A2"); + Path dst = new Path("/hbase/A1/A3"); + + // Create sample files in the source directory + createFiles(currentFs, src, TOTAL_FILES); + + // Test renaming with different configurations + renameDir(currentFs, "10", "5", "2", src, dst); + renameDir(currentFs, "100", "5", "2", dst, src); + + String errorMessage = intercept(PathIOException.class, + () -> renameDir(currentFs, "50", "50", "5", src, dst)) + .getMessage(); + + // Validate error message for invalid configuration + Assertions.assertThat(errorMessage) + .describedAs("maxConsumptionLag should be lesser than maxSize") + .contains( + "Invalid configuration value detected for \"fs.azure.blob.dir.list.consumer.max.lag\". " + + "maxConsumptionLag should be lesser than maxSize"); + } + } + + /** + * Helper method to create files in the given directory. + * + * @param fs The AzureBlobFileSystem instance to use for file creation. + * @param src The source path (directory). + * @param numFiles The number of files to create. + * @throws ExecutionException, InterruptedException If an error occurs during file creation. + */ + private void createFiles(AzureBlobFileSystem fs, Path src, int numFiles) + throws ExecutionException, InterruptedException { + ExecutorService executorService = Executors.newFixedThreadPool(TOTAL_THREADS_IN_POOL); + List futures = new ArrayList<>(); + for (int i = 0; i < numFiles; i++) { + final int iter = i; + Future future = executorService.submit(() -> + fs.create(new Path(src, "file" + iter + ".txt"))); + futures.add(future); + } + for (Future future : futures) { + future.get(); + } + executorService.shutdown(); + } + + /** + * Tests renaming a directory with a failure during the copy operation. + * Simulates an error when copying on the 6th call. + */ + @Test + public void testRenameCopyFailureInBetween() throws Exception { + try (final AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem( + createConfig("5", "3", "2")))) { + assumeBlobServiceType(); + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + fs.getAbfsStore().setClient(client); + Path src = new Path("/hbase/A1/A2"); + Path dst = new Path("/hbase/A1/A3"); + + // Create sample files in the source directory + createFiles(fs, src, TOTAL_FILES); + + // Track the number of copy operations + AtomicInteger copyCall = new AtomicInteger(0); + Mockito.doAnswer(copyRequest -> { + if (copyCall.get() == FAILED_CALL) { + throw new AbfsRestOperationException( + BLOB_ALREADY_EXISTS.getStatusCode(), + BLOB_ALREADY_EXISTS.getErrorCode(), + BLOB_ALREADY_EXISTS.getErrorMessage(), + new Exception()); + } + copyCall.incrementAndGet(); + return copyRequest.callRealMethod(); + }).when(client).copyBlob(Mockito.any(Path.class), + Mockito.any(Path.class), Mockito.nullable(String.class), + Mockito.any(TracingContext.class)); + + fs.rename(src, dst); + // Validate copy operation count + Assertions.assertThat(copyCall.get()) + .describedAs("Copy operation count should be less than 10.") + .isLessThan(TOTAL_FILES); + + // Validate that rename redo operation was triggered + copyCall.set(0); + + // Assertions to validate renamed destination and source + validateRename(fs, src, dst, false, true, true); + + Assertions.assertThat(copyCall.get()) + .describedAs("Copy operation count should be greater than 0.") + .isGreaterThan(0); + + // Validate final state of destination and source + validateRename(fs, src, dst, false, true, false); + } + } + + /** + * Tests renaming a directory with a failure during the delete operation. + * Simulates an error on the 6th delete operation and verifies the behavior. + */ + @Test + public void testRenameDeleteFailureInBetween() throws Exception { + try (final AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem( + createConfig("5", "3", "2")))) { + assumeBlobServiceType(); + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + fs.getAbfsStore().setClient(client); + Path src = new Path("/hbase/A1/A2"); + Path dst = new Path("/hbase/A1/A3"); + + // Create sample files in the source directory + createFiles(fs, src, TOTAL_FILES); + + // Track the number of delete operations + AtomicInteger deleteCall = new AtomicInteger(0); + Mockito.doAnswer(deleteRequest -> { + if (deleteCall.get() == FAILED_CALL) { + throw new AbfsRestOperationException( + BLOB_PATH_NOT_FOUND.getStatusCode(), + BLOB_PATH_NOT_FOUND.getErrorCode(), + BLOB_PATH_NOT_FOUND.getErrorMessage(), + new Exception()); + } + deleteCall.incrementAndGet(); + return deleteRequest.callRealMethod(); + }).when(client).deleteBlobPath(Mockito.any(Path.class), + Mockito.anyString(), Mockito.any(TracingContext.class)); + + fs.rename(src, dst); + + // Validate delete operation count + Assertions.assertThat(deleteCall.get()) + .describedAs("Delete operation count should be less than 10.") + .isLessThan(TOTAL_FILES); + + // Validate that delete redo operation was triggered + deleteCall.set(0); + // Assertions to validate renamed destination and source + validateRename(fs, src, dst, false, true, true); + + Assertions.assertThat(deleteCall.get()) + .describedAs("Delete operation count should be greater than 0.") + .isGreaterThan(0); + + // Validate final state of destination and source + // Validate that delete redo operation was triggered + validateRename(fs, src, dst, false, true, false); + } + } + +// /** +// * Tests renaming a file or directory when the destination path contains +// * a colon (":"). The test ensures that: +// * - The source directory exists before the rename. +// * - The file is successfully renamed to the destination path. +// * - The old source directory no longer exists after the rename. +// * - The new destination directory exists after the rename. +// * +// * @throws Exception if an error occurs during file system operations +// */ +// @Test +// public void testRenameWhenDestinationPathContainsColon() throws Exception { +// AzureBlobFileSystem fs = getFileSystem(); +// fs.setWorkingDirectory(new Path(ROOT_PATH)); +// String fileName = "file"; +// Path src = new Path("/test1/"); +// Path dst = new Path("/test1:/"); +// +// // Create the file +// fs.create(new Path(src, fileName)); +// +// // Perform the rename operation and validate the results +// performRenameAndValidate(fs, src, dst, fileName); +// } + +// /** +// * Performs the rename operation and validates the existence of the directories and files. +// * +// * @param fs the AzureBlobFileSystem instance +// * @param src the source path to be renamed +// * @param dst the destination path for the rename +// * @param fileName the name of the file to be renamed +// */ +// private void performRenameAndValidate(AzureBlobFileSystem fs, Path src, Path dst, String fileName) +// throws IOException { +// // Assert the source directory exists +// Assertions.assertThat(fs.exists(src)) +// .describedAs("Old directory should exist before rename") +// .isTrue(); +// +// // Perform rename +// fs.rename(src, dst); +// +// // Assert the destination directory and file exist after rename +// Assertions.assertThat(fs.exists(new Path(dst, fileName))) +// .describedAs("Rename should be successful") +// .isTrue(); +// +// // Assert the source directory no longer exists +// Assertions.assertThat(fs.exists(src)) +// .describedAs("Old directory should not exist") +// .isFalse(); +// +// // Assert the new destination directory exists +// Assertions.assertThat(fs.exists(dst)) +// .describedAs("New directory should exist") +// .isTrue(); +// } + + /** + * Tests the behavior of the atomic rename key for the root folder + * in Azure Blob File System. The test verifies that the atomic rename key + * returns false for the root folder path. + * + * @throws Exception if an error occurs during the atomic rename key check + */ + @Test + public void testGetAtomicRenameKeyForRootFolder() throws Exception { + AzureBlobFileSystem fs = getFileSystem(); + assumeBlobServiceType(); + AbfsBlobClient abfsBlobClient = (AbfsBlobClient) fs.getAbfsClient(); + Assertions.assertThat(abfsBlobClient.isAtomicRenameKey("/hbase")) + .describedAs("Atomic rename key should return false for Root folder") + .isFalse(); + } + + /** + * Tests the behavior of the atomic rename key for non-root folders + * in Azure Blob File System. The test verifies that the atomic rename key + * works for specific folders as defined in the configuration. + * It checks the atomic rename key for various paths, + * ensuring it returns true for matching paths and false for others. + * + * @throws Exception if an error occurs during the atomic rename key check + */ + @Test + public void testGetAtomicRenameKeyForNonRootFolder() throws Exception { + final AzureBlobFileSystem currentFs = getFileSystem(); + Configuration config = new Configuration(this.getRawConfiguration()); + config.set(FS_AZURE_ATOMIC_RENAME_KEY, "/hbase,/a,/b"); + + final AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(currentFs.getUri(), config); + assumeBlobServiceType(); + AbfsBlobClient abfsBlobClient = (AbfsBlobClient) fs.getAbfsClient(); + + // Test for various paths + validateAtomicRenameKey(abfsBlobClient, "/hbase1/test", false); + validateAtomicRenameKey(abfsBlobClient, "/hbase/test", true); + validateAtomicRenameKey(abfsBlobClient, "/a/b/c", true); + validateAtomicRenameKey(abfsBlobClient, "/test/a", false); + } + + /** + * Validates the atomic rename key for a specific path. + * + * @param abfsBlobClient the AbfsBlobClient instance + * @param path the path to check for atomic rename key + * @param expected the expected value (true or false) + */ + private void validateAtomicRenameKey(AbfsBlobClient abfsBlobClient, String path, boolean expected) { + Assertions.assertThat(abfsBlobClient.isAtomicRenameKey(path)) + .describedAs("Atomic rename key check for path: " + path) + .isEqualTo(expected); + } + + /** + * Helper method to create a json file. + * @param path parent path + * @param renameJson rename json path + * @return file system + * @throws IOException in case of failure + */ + public AzureBlobFileSystem createJsonFile(Path path, Path renameJson) throws IOException { + final AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem()); + assumeBlobServiceType(); + AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore()); + Mockito.doReturn(store).when(fs).getAbfsStore(); + AbfsClient client = Mockito.spy(store.getClient()); + Mockito.doReturn(client).when(store).getClient(); + + fs.setWorkingDirectory(new Path(ROOT_PATH)); + fs.create(new Path(path, "file.txt")); + + AzureBlobFileSystemStore.VersionedFileStatus fileStatus = (AzureBlobFileSystemStore.VersionedFileStatus) fs.getFileStatus(path); + + new RenameAtomicity(path, new Path("/hbase/test4"), renameJson, getTestTracingContext(fs, true), fileStatus.getEtag(), client) + .preRename(); + + Assertions.assertThat(fs.exists(renameJson)) + .describedAs("Rename Pending Json file should exist.") + .isTrue(); + + return fs; + } + + /** + * Test case to verify crash recovery with a single child folder. + * + * This test simulates a scenario where a pending rename JSON file exists for a single child folder + * under the parent directory. It ensures that when listing the files in the parent directory, + * only the child folder (with the pending rename JSON file) is returned, and no additional files are listed. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void listCrashRecoveryWithSingleChildFolder() throws Exception { + AzureBlobFileSystem fs = null; + try { + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs = createJsonFile(path, renameJson); + + FileStatus[] fileStatuses = fs.listStatus(new Path("/hbase/A1")); + + Assertions.assertThat(fileStatuses.length) + .describedAs("List should return 1 file") + .isEqualTo(1); + } finally { + if (fs != null) { + fs.close(); + } + } + } + + /** + * Test case to verify crash recovery with multiple child folders. + * + * This test simulates a scenario where a pending rename JSON file exists, and multiple files are + * created in the parent directory. It ensures that when listing the files in the parent directory, + * the correct number of files is returned, including the pending rename JSON file. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void listCrashRecoveryWithMultipleChildFolder() throws Exception { + AzureBlobFileSystem fs = null; + try { + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs = createJsonFile(path, renameJson); + + fs.create(new Path("/hbase/A1/file1.txt")); + fs.create(new Path("/hbase/A1/file2.txt")); + + FileStatus[] fileStatuses = fs.listStatus(new Path("/hbase/A1")); + + Assertions.assertThat(fileStatuses.length) + .describedAs("List should return 3 files") + .isEqualTo(3); + } finally { + if (fs != null) { + fs.close(); + } + } + } + + /** + * Test case to verify crash recovery with a pending rename JSON file. + * + * This test simulates a scenario where a pending rename JSON file exists in the parent directory, + * and it ensures that after the deletion of the target directory and creation of new files, + * the listing operation correctly returns the remaining files without considering the pending rename. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void listCrashRecoveryWithPendingJsonFile() throws Exception { + AzureBlobFileSystem fs = null; + try { + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs = createJsonFile(path, renameJson); + + fs.delete(path, true); + fs.create(new Path("/hbase/A1/file1.txt")); + fs.create(new Path("/hbase/A1/file2.txt")); + + FileStatus[] fileStatuses = fs.listStatus(path.getParent()); + + Assertions.assertThat(fileStatuses.length) + .describedAs("List should return 2 files") + .isEqualTo(2); + } finally { + if (fs != null) { + fs.close(); + } + } + } + + /** + * Test case to verify crash recovery when no pending rename JSON file exists. + * + * This test simulates a scenario where there is no pending rename JSON file in the directory. + * It ensures that the listing operation correctly returns all files in the parent directory, including + * those created after the rename JSON file is deleted. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void listCrashRecoveryWithoutAnyPendingJsonFile() throws Exception { + AzureBlobFileSystem fs = null; + try { + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs = createJsonFile(path, renameJson); + + fs.delete(renameJson, true); + fs.create(new Path("/hbase/A1/file1.txt")); + fs.create(new Path("/hbase/A1/file2.txt")); + + FileStatus[] fileStatuses = fs.listStatus(path.getParent()); + + Assertions.assertThat(fileStatuses.length) + .describedAs("List should return 3 files") + .isEqualTo(3); + } finally { + if (fs != null) { + fs.close(); + } + } + } + + /** + * Test case to verify crash recovery when a pending rename JSON directory exists. + * + * This test simulates a scenario where a pending rename JSON directory exists, ensuring that the + * listing operation correctly returns all files in the parent directory without triggering a redo + * rename operation. It also checks that the directory with the suffix "-RenamePending.json" exists. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void listCrashRecoveryWithPendingJsonDir() throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem())) { + assumeBlobServiceType(); + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs.mkdirs(renameJson); + + fs.create(new Path(path.getParent(), "file1.txt")); + fs.create(new Path(path, "file2.txt")); + + AtomicInteger redoRenameCall = new AtomicInteger(0); + Mockito.doAnswer(answer -> { + redoRenameCall.incrementAndGet(); + return answer.callRealMethod(); + }).when(client).getRedoRenameAtomicity(Mockito.any(Path.class), + Mockito.anyInt(), Mockito.any(TracingContext.class)); + + FileStatus[] fileStatuses = fs.listStatus(path.getParent()); + + Assertions.assertThat(fileStatuses.length) + .describedAs("List should return 3 files") + .isEqualTo(3); + + Assertions.assertThat(redoRenameCall.get()) + .describedAs("No redo rename call should be made") + .isEqualTo(0); + + Assertions.assertThat( + Arrays.stream(fileStatuses) + .anyMatch(status -> renameJson.toUri().getPath().equals(status.getPath().toUri().getPath()))) + .describedAs("Directory with suffix -RenamePending.json should exist.") + .isTrue(); + } + } + + /** + * Test case to verify crash recovery during listing with multiple pending rename JSON files. + * + * This test simulates a scenario where multiple pending rename JSON files exist, ensuring that + * crash recovery properly handles the situation. It verifies that two redo rename calls are made + * and that the list operation returns the correct number of paths. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void listCrashRecoveryWithMultipleJsonFile() throws Exception { + AzureBlobFileSystem fs = null; + try { + Path path = new Path("/hbase/A1/A2"); + + // 1st Json file + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs = createJsonFile(path, renameJson); + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + + // 2nd Json file + Path path2 = new Path("/hbase/A1/A3"); + fs.create(new Path(path2, "file3.txt")); + + Path renameJson2 = new Path(path2.getParent(), path2.getName() + SUFFIX); + AzureBlobFileSystemStore.VersionedFileStatus fileStatus = (AzureBlobFileSystemStore.VersionedFileStatus) fs.getFileStatus(path2); + + new RenameAtomicity(path2, new Path("/hbase/test4"), renameJson2, getTestTracingContext(fs, true), fileStatus.getEtag(), client).preRename(); + + fs.create(new Path(path, "file2.txt")); + + AtomicInteger redoRenameCall = new AtomicInteger(0); + Mockito.doAnswer(answer -> { + redoRenameCall.incrementAndGet(); + return answer.callRealMethod(); + }).when(client).getRedoRenameAtomicity(Mockito.any(Path.class), + Mockito.anyInt(), Mockito.any(TracingContext.class)); + + FileStatus[] fileStatuses = fs.listStatus(path.getParent()); + + Assertions.assertThat(fileStatuses.length) + .describedAs("List should return 2 paths") + .isEqualTo(2); + + Assertions.assertThat(redoRenameCall.get()) + .describedAs("2 redo rename calls should be made") + .isEqualTo(2); + } finally { + if (fs != null) { + fs.close(); + } + } + } + + /** + * Test case to verify path status when a pending rename JSON file exists. + * + * This test simulates a scenario where a rename operation was pending, and ensures that + * the path status retrieval triggers a redo rename operation. The test also checks that + * the correct error code (`PATH_NOT_FOUND`) is returned. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void getPathStatusWithPendingJsonFile() throws Exception { + AzureBlobFileSystem fs = null; + try { + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + fs = createJsonFile(path, renameJson); + + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + + fs.create(new Path("/hbase/A1/file1.txt")); + fs.create(new Path("/hbase/A1/file2.txt")); + + AbfsConfiguration conf = fs.getAbfsStore().getAbfsConfiguration(); + + AtomicInteger redoRenameCall = new AtomicInteger(0); + Mockito.doAnswer(answer -> { + redoRenameCall.incrementAndGet(); + return answer.callRealMethod(); + }).when(client).getRedoRenameAtomicity(Mockito.any(Path.class), + Mockito.anyInt(), Mockito.any(TracingContext.class)); + + TracingContext tracingContext = new TracingContext( + conf.getClientCorrelationId(), fs.getFileSystemId(), + FSOperationType.GET_FILESTATUS, TracingHeaderFormat.ALL_ID_FORMAT, null); + + AzureServiceErrorCode azureServiceErrorCode = intercept( + AbfsRestOperationException.class, () -> client.getPathStatus( + path.toUri().getPath(), true, + tracingContext, null)).getErrorCode(); + + Assertions.assertThat(azureServiceErrorCode.getErrorCode()) + .describedAs("Path had to be recovered from atomic rename operation.") + .isEqualTo(PATH_NOT_FOUND.getErrorCode()); + + Assertions.assertThat(redoRenameCall.get()) + .describedAs("There should be one redo rename call") + .isEqualTo(1); + } finally { + if (fs != null) { + fs.close(); + } + } + } + + /** + * Test case to verify path status when there is no pending rename JSON file. + * + * This test ensures that when no rename pending JSON file is present, the path status is + * successfully retrieved, the ETag is present, and no redo rename operation is triggered. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void getPathStatusWithoutPendingJsonFile() throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem())) { + assumeBlobServiceType(); + + Path path = new Path("/hbase/A1/A2"); + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + + fs.create(new Path(path, "file1.txt")); + fs.create(new Path(path, "file2.txt")); + + AbfsConfiguration conf = fs.getAbfsStore().getAbfsConfiguration(); + + AtomicInteger redoRenameCall = new AtomicInteger(0); + Mockito.doAnswer(answer -> { + redoRenameCall.incrementAndGet(); + return answer.callRealMethod(); + }).when(client).getRedoRenameAtomicity( + Mockito.any(Path.class), Mockito.anyInt(), + Mockito.any(TracingContext.class)); + + TracingContext tracingContext = new TracingContext( + conf.getClientCorrelationId(), fs.getFileSystemId(), + FSOperationType.GET_FILESTATUS, TracingHeaderFormat.ALL_ID_FORMAT, + null); + + AbfsHttpOperation abfsHttpOperation = client.getPathStatus( + path.toUri().getPath(), true, + tracingContext, null).getResult(); + + Assertions.assertThat(abfsHttpOperation.getStatusCode()) + .describedAs("Path should be found.") + .isEqualTo(HTTP_OK); + + Assertions.assertThat(extractEtagHeader(abfsHttpOperation)) + .describedAs("Etag should be present.") + .isNotNull(); + + Assertions.assertThat(redoRenameCall.get()) + .describedAs("There should be no redo rename call.") + .isEqualTo(0); + } + } + + /** + * Test case to verify path status when there is a pending rename JSON directory. + * + * This test simulates the scenario where a directory is created with a rename pending JSON + * file (indicated by a specific suffix). It ensures that the path is found, the ETag is present, + * and no redo rename operation is triggered. It also verifies that the rename pending directory + * exists. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void getPathStatusWithPendingJsonDir() throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem())) { + assumeBlobServiceType(); + + Path path = new Path("/hbase/A1/A2"); + AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); + + fs.create(new Path(path, "file1.txt")); + fs.create(new Path(path, "file2.txt")); + + fs.mkdirs(new Path(path.getParent(), path.getName() + SUFFIX)); + + AbfsConfiguration conf = fs.getAbfsStore().getAbfsConfiguration(); + + AtomicInteger redoRenameCall = new AtomicInteger(0); + Mockito.doAnswer(answer -> { + redoRenameCall.incrementAndGet(); + return answer.callRealMethod(); + }).when(client).getRedoRenameAtomicity(Mockito.any(Path.class), + Mockito.anyInt(), Mockito.any(TracingContext.class)); + + TracingContext tracingContext = new TracingContext( + conf.getClientCorrelationId(), fs.getFileSystemId(), + FSOperationType.GET_FILESTATUS, TracingHeaderFormat.ALL_ID_FORMAT, null); + + AbfsHttpOperation abfsHttpOperation = client.getPathStatus(path.toUri().getPath(), true, tracingContext, null).getResult(); + + Assertions.assertThat(abfsHttpOperation.getStatusCode()) + .describedAs("Path should be found.") + .isEqualTo(HTTP_OK); + + Assertions.assertThat(extractEtagHeader(abfsHttpOperation)) + .describedAs("Etag should be present.") + .isNotNull(); + + Assertions.assertThat(redoRenameCall.get()) + .describedAs("There should be no redo rename call.") + .isEqualTo(0); + + Assertions.assertThat(fs.exists(new Path(path.getParent(), path.getName() + SUFFIX))) + .describedAs("Directory with suffix -RenamePending.json should exist.") + .isTrue(); + } + } + + /** + * Test case to verify the behavior when the ETag of a file changes during a rename operation. + * + * This test simulates a scenario where the ETag of a file changes after the creation of a + * rename pending JSON file. The steps include: + * - Creating a rename pending JSON file with an old ETag. + * - Deleting the original directory for an ETag change. + * - Creating new files in the directory. + * - Verifying that the copy blob call is not triggered. + * - Verifying that the rename atomicity operation is called once. + * + * The test ensures that the system correctly handles the ETag change during the rename process. + * + * @throws Exception if any error occurs during the test execution + */ + @Test + public void eTagChangedDuringRename() throws Exception { + AzureBlobFileSystem fs = null; + try { + assumeBlobServiceType(); + Path path = new Path("/hbase/A1/A2"); + Path renameJson = new Path(path.getParent(), path.getName() + SUFFIX); + // Create rename pending json file with old etag + fs = createJsonFile(path, renameJson); + AbfsBlobClient abfsBlobClient = (AbfsBlobClient) addSpyHooksOnClient(fs); + fs.getAbfsStore().setClient(abfsBlobClient); + + // Delete the directory to change etag + fs.delete(path, true); + + fs.create(new Path(path, "file1.txt")); + fs.create(new Path(path, "file2.txt")); + AtomicInteger numberOfCopyBlobCalls = new AtomicInteger(0); + Mockito.doAnswer( copyBlob -> { + numberOfCopyBlobCalls.incrementAndGet(); + return copyBlob.callRealMethod(); + }) + .when(abfsBlobClient) + .copyBlob(Mockito.any(Path.class), Mockito.any(Path.class), + Mockito.nullable(String.class), + Mockito.any(TracingContext.class)); + + AtomicInteger numberOfRedoRenameAtomicityCalls = new AtomicInteger(0); + Mockito.doAnswer( redoRenameAtomicity -> { + numberOfRedoRenameAtomicityCalls.incrementAndGet(); + return redoRenameAtomicity.callRealMethod(); + }) + .when(abfsBlobClient) + .getRedoRenameAtomicity(Mockito.any(Path.class), Mockito.anyInt(), + Mockito.any(TracingContext.class)); + // Call list status to trigger rename redo + fs.listStatus(path.getParent()); + Assertions.assertThat(numberOfRedoRenameAtomicityCalls.get()) + .describedAs("There should be one call to getRedoRenameAtomicity") + .isEqualTo(1); + Assertions.assertThat(numberOfCopyBlobCalls.get()) + .describedAs("There should be no copy blob call") + .isEqualTo(0); + } finally { + if (fs != null) { + fs.close(); + } + } + } } From 35b31f95f614c22e990cddad3450bf1f7d448fda Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Wed, 19 Feb 2025 06:49:19 -0800 Subject: [PATCH 2/6] Test cases fixes for rename --- .../fs/azurebfs/AzureBlobFileSystem.java | 2 - .../services/AzureServiceErrorCode.java | 1 - .../fs/azurebfs/services/AbfsDfsClient.java | 2 + .../azurebfs/services/BlobRenameHandler.java | 25 +-- .../fs/azurebfs/services/ListActionTaker.java | 12 +- .../ITestAzureBlobFileSystemRename.java | 177 ++++++++++++------ ...ITestAzureBlobFileSystemRenameUnicode.java | 17 -- 7 files changed, 129 insertions(+), 107 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 e45603c278d25..30b9d315ba902 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 @@ -993,8 +993,6 @@ public Void call() throws Exception { delete(fs.getPath(), fs.isDirectory()); if (fs.isDirectory()) { statIncrement(DIRECTORIES_DELETED); - } else { - statIncrement(FILES_DELETED); } return null; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java index 87949e36b34f3..2dcaa42debb1a 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java @@ -56,7 +56,6 @@ public enum AzureServiceErrorCode { OTHER_SERVER_THROTTLING("ServerBusy", HttpURLConnection.HTTP_UNAVAILABLE, "The server is currently unable to receive requests. Please retry your request."), INVALID_QUERY_PARAMETER_VALUE("InvalidQueryParameterValue", HttpURLConnection.HTTP_BAD_REQUEST, null), - INVALID_RENAME_DESTINATION("InvalidRenameDestinationPath", HttpURLConnection.HTTP_BAD_REQUEST, null), AUTHORIZATION_PERMISSION_MISS_MATCH("AuthorizationPermissionMismatch", HttpURLConnection.HTTP_FORBIDDEN, null), ACCOUNT_REQUIRES_HTTPS("AccountRequiresHttps", HttpURLConnection.HTTP_BAD_REQUEST, null), MD5_MISMATCH("Md5Mismatch", HttpURLConnection.HTTP_BAD_REQUEST, diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java index 8505b533ce4c4..82f38b94929c6 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java @@ -1161,6 +1161,8 @@ public AbfsRestOperation deletePath(final String path, } else { return idempotencyOp; } + } finally { + incrementAbfsDeleteFile(); } return op; diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java index b9583e1fb41cf..88f736bbb86d7 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java @@ -31,7 +31,6 @@ import org.apache.hadoop.classification.VisibleForTesting; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.PathIOException; 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.exceptions.TimeoutException; @@ -258,7 +257,7 @@ private boolean containsColon(Path p) { private boolean preCheck(final Path src, final Path dst, final PathInformation pathInformation) throws AzureBlobFileSystemException { - validateDestinationPath(src, dst); + validateDestinationIsNotSubDir(src, dst); validateSourcePath(pathInformation); validateDestinationPathNotExist(src, dst, pathInformation); validateDestinationParentExist(src, dst, pathInformation); @@ -266,28 +265,6 @@ private boolean preCheck(final Path src, final Path dst, return true; } - /** - * Validate if the format of the destination path is correct and if the destination - * path is not a sub-directory of the source path. - * - * @param src source path - * @param dst destination path - * - * @throws AbfsRestOperationException if the destination path is invalid - */ - private void validateDestinationPath(final Path src, final Path dst) - throws AbfsRestOperationException { - if (containsColon(dst)) { - throw new AbfsRestOperationException( - HttpURLConnection.HTTP_BAD_REQUEST, - AzureServiceErrorCode.INVALID_RENAME_DESTINATION.getErrorCode(), null, - new PathIOException(dst.toUri().getPath(), - "Destination path contains colon")); - } - - validateDestinationIsNotSubDir(src, dst); - } - /** * Validate if the destination path is not a sub-directory of the source path. * diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ListActionTaker.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ListActionTaker.java index b87f16ae46107..ed3d464e9b605 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ListActionTaker.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ListActionTaker.java @@ -35,6 +35,7 @@ import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; 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.exceptions.FileSystemOperationUnhandledException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException; import org.apache.hadoop.fs.azurebfs.contracts.services.BlobListResultSchema; import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultEntrySchema; @@ -119,7 +120,14 @@ private boolean takeAction(List paths) LOG.debug("Thread interrupted while taking action on path: {}", path.toUri().getPath()); } catch (ExecutionException e) { - executionException = (AzureBlobFileSystemException) e.getCause(); + LOG.debug("Execution exception while taking action on path: {}", + path.toUri().getPath()); + if (e.getCause() instanceof AzureBlobFileSystemException) { + executionException = (AzureBlobFileSystemException) e.getCause(); + } else { + executionException = + new FileSystemOperationUnhandledException(executionException); + } } } if (executionException != null) { @@ -261,7 +269,7 @@ protected String listAndEnqueue(final ListBlobQueue listBlobQueue, protected void addPaths(final List paths, final ListResultSchema retrievedSchema) { for (ListResultEntrySchema entry : retrievedSchema.paths()) { - Path entryPath = new Path(ROOT_PATH, entry.name()); + Path entryPath = new Path(ROOT_PATH + entry.name()); if (!entryPath.equals(this.path)) { paths.add(entryPath); } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index 63593ce94d79f..9d2d1e8837ded 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -311,12 +311,67 @@ public void testRenameNotFoundBlobToEmptyRoot() throws Exception { * * @throws Exception if an error occurs during test execution */ - @Test(expected = IOException.class) - public void testRenameBlobToDstWithColonInPath() throws Exception { + @Test + public void testRenameBlobToDstWithColonInSourcePath() throws Exception { AzureBlobFileSystem fs = getFileSystem(); assumeBlobServiceType(); + fs.create(new Path("/src:/file")); + Assertions.assertThat( + fs.rename(new Path("/src:"), + new Path("/dst")) + ).isTrue(); + } + + /** + * Tests renaming a source path to a destination path that contains a colon in the path. + * This verifies that the rename operation handles paths with special characters like a colon. + * + * The test creates a source directory and renames it to a destination path that includes a colon, + * ensuring that the operation succeeds without errors. + * + * @throws Exception if an error occurs during test execution + */ + @Test + public void testRenameWithColonInDestinationPath() throws Exception { + AzureBlobFileSystem fs = getFileSystem(); fs.create(new Path("/src")); - fs.rename(new Path("/src"), new Path("/dst:file")); + Assertions.assertThat( + fs.rename(new Path("/src"), + new Path("/dst:")) + ).isTrue(); + } + + @Test + public void testRenameWithColonInSourcePath() throws Exception { + AzureBlobFileSystem fs = getFileSystem(); + String sourceDirectory = "/src:"; + String destinationDirectory = "/dst"; + String fileName = "file"; + fs.create(new Path(sourceDirectory, fileName)); + fs.create(new Path(sourceDirectory + "/Test:", fileName)); + // Rename from source to destination + Assertions.assertThat( + fs.rename(new Path(sourceDirectory), + new Path(destinationDirectory)) + ).isTrue(); + Assertions.assertThat( + fs.exists(new Path(sourceDirectory, fileName))) + .isFalse(); + Assertions.assertThat( + fs.exists(new Path(destinationDirectory, fileName))) + .isTrue(); + + // Rename from destination to source + Assertions.assertThat( + fs.rename(new Path(destinationDirectory), + new Path(sourceDirectory)) + ).isTrue(); + Assertions.assertThat( + fs.exists(new Path(sourceDirectory, fileName))) + .isTrue(); + Assertions.assertThat( + fs.exists(new Path(destinationDirectory, fileName))) + .isFalse(); } /** @@ -1883,64 +1938,64 @@ public void testRenameDeleteFailureInBetween() throws Exception { } } -// /** -// * Tests renaming a file or directory when the destination path contains -// * a colon (":"). The test ensures that: -// * - The source directory exists before the rename. -// * - The file is successfully renamed to the destination path. -// * - The old source directory no longer exists after the rename. -// * - The new destination directory exists after the rename. -// * -// * @throws Exception if an error occurs during file system operations -// */ -// @Test -// public void testRenameWhenDestinationPathContainsColon() throws Exception { -// AzureBlobFileSystem fs = getFileSystem(); -// fs.setWorkingDirectory(new Path(ROOT_PATH)); -// String fileName = "file"; -// Path src = new Path("/test1/"); -// Path dst = new Path("/test1:/"); -// -// // Create the file -// fs.create(new Path(src, fileName)); -// -// // Perform the rename operation and validate the results -// performRenameAndValidate(fs, src, dst, fileName); -// } - -// /** -// * Performs the rename operation and validates the existence of the directories and files. -// * -// * @param fs the AzureBlobFileSystem instance -// * @param src the source path to be renamed -// * @param dst the destination path for the rename -// * @param fileName the name of the file to be renamed -// */ -// private void performRenameAndValidate(AzureBlobFileSystem fs, Path src, Path dst, String fileName) -// throws IOException { -// // Assert the source directory exists -// Assertions.assertThat(fs.exists(src)) -// .describedAs("Old directory should exist before rename") -// .isTrue(); -// -// // Perform rename -// fs.rename(src, dst); -// -// // Assert the destination directory and file exist after rename -// Assertions.assertThat(fs.exists(new Path(dst, fileName))) -// .describedAs("Rename should be successful") -// .isTrue(); -// -// // Assert the source directory no longer exists -// Assertions.assertThat(fs.exists(src)) -// .describedAs("Old directory should not exist") -// .isFalse(); -// -// // Assert the new destination directory exists -// Assertions.assertThat(fs.exists(dst)) -// .describedAs("New directory should exist") -// .isTrue(); -// } + /** + * Tests renaming a file or directory when the destination path contains + * a colon (":"). The test ensures that: + * - The source directory exists before the rename. + * - The file is successfully renamed to the destination path. + * - The old source directory no longer exists after the rename. + * - The new destination directory exists after the rename. + * + * @throws Exception if an error occurs during file system operations + */ + @Test + public void testRenameWhenDestinationPathContainsColon() throws Exception { + AzureBlobFileSystem fs = getFileSystem(); + fs.setWorkingDirectory(new Path(ROOT_PATH)); + String fileName = "file"; + Path src = new Path("/test1/"); + Path dst = new Path("/test1:/"); + + // Create the file + fs.create(new Path(src, fileName)); + + // Perform the rename operation and validate the results + performRenameAndValidate(fs, src, dst, fileName); + } + + /** + * Performs the rename operation and validates the existence of the directories and files. + * + * @param fs the AzureBlobFileSystem instance + * @param src the source path to be renamed + * @param dst the destination path for the rename + * @param fileName the name of the file to be renamed + */ + private void performRenameAndValidate(AzureBlobFileSystem fs, Path src, Path dst, String fileName) + throws IOException { + // Assert the source directory exists + Assertions.assertThat(fs.exists(src)) + .describedAs("Old directory should exist before rename") + .isTrue(); + + // Perform rename + fs.rename(src, dst); + + // Assert the destination directory and file exist after rename + Assertions.assertThat(fs.exists(new Path(dst, fileName))) + .describedAs("Rename should be successful") + .isTrue(); + + // Assert the source directory no longer exists + Assertions.assertThat(fs.exists(src)) + .describedAs("Old directory should not exist") + .isFalse(); + + // Assert the new destination directory exists + Assertions.assertThat(fs.exists(dst)) + .describedAs("New directory should exist") + .isTrue(); + } /** * Tests the behavior of the atomic rename key for the root folder diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRenameUnicode.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRenameUnicode.java index b134c0e93bd31..589ca5415fc80 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRenameUnicode.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRenameUnicode.java @@ -24,21 +24,15 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; -import org.apache.hadoop.fs.azurebfs.constants.AbfsServiceType; -import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.PathIOException; -import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.COLON; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsDirectory; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsFile; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertRenameOutcome; -import static org.apache.hadoop.test.LambdaTestUtils.intercept; /** * Parameterized test of rename operations of unicode paths. @@ -90,17 +84,6 @@ public void testRenameFileUsingUnicode() throws Exception { assertIsFile(fs, filePath); Path folderPath2 = new Path(destDir); - if (getAbfsServiceType() == AbfsServiceType.BLOB - && destDir.contains(COLON)) { - AbfsRestOperationException ex = intercept( - AbfsRestOperationException.class, () -> { - fs.rename(folderPath1, folderPath2); - return null; - }); - assertTrue(ex.getCause() instanceof PathIOException); - assertEquals(HTTP_BAD_REQUEST, ex.getStatusCode()); - return; - } assertRenameOutcome(fs, folderPath1, folderPath2, true); assertPathDoesNotExist(fs, "renamed", folderPath1); assertIsDirectory(fs, folderPath2); From 6f5a3a37baa5788f77e57331c413ec49f2bfcbfe Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Wed, 19 Feb 2025 06:53:05 -0800 Subject: [PATCH 3/6] Checkstyle fixes --- .../hadoop/fs/azurebfs/services/AbfsBlobClient.java | 1 - .../hadoop/fs/azurebfs/services/AbfsClient.java | 2 -- .../fs/azurebfs/ITestAzureBlobFileSystemRename.java | 12 ++++++------ 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java index a9c06526f9a9d..bd08b30d3150c 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java @@ -84,7 +84,6 @@ import static java.net.HttpURLConnection.HTTP_OK; import static java.net.HttpURLConnection.HTTP_PRECON_FAILED; import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ACQUIRE_LEASE_ACTION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.AND_MARK; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APPEND_BLOB_TYPE; 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 754487b62cd7c..a397a607059c5 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 @@ -92,8 +92,6 @@ import static org.apache.commons.lang3.StringUtils.isNotEmpty; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_CREATED; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_COPIED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_CREATED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_DELETED; diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index 9d2d1e8837ded..08d235d96c3ea 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -1732,7 +1732,7 @@ private void renameDir(AzureBlobFileSystem currentFs, String producerQueueSize, String consumerMaxLag, String maxThread, Path src, Path dst) throws IOException { Configuration config = createConfig(producerQueueSize, consumerMaxLag, maxThread); - try (final AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(currentFs.getUri(), config)) { + try (AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(currentFs.getUri(), config)) { fs.rename(src, dst); validateRename(fs, src, dst, false, true, false); } @@ -1781,7 +1781,7 @@ private void validateRename(AzureBlobFileSystem fs, Path src, Path dst, */ @Test public void testRenameDirWithDifferentParallelism() throws Exception { - try (final AzureBlobFileSystem currentFs = getFileSystem()) { + try (AzureBlobFileSystem currentFs = getFileSystem()) { assumeBlobServiceType(); Path src = new Path("/hbase/A1/A2"); Path dst = new Path("/hbase/A1/A3"); @@ -1836,7 +1836,7 @@ private void createFiles(AzureBlobFileSystem fs, Path src, int numFiles) */ @Test public void testRenameCopyFailureInBetween() throws Exception { - try (final AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem( + try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem( createConfig("5", "3", "2")))) { assumeBlobServiceType(); AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); @@ -1890,7 +1890,7 @@ public void testRenameCopyFailureInBetween() throws Exception { */ @Test public void testRenameDeleteFailureInBetween() throws Exception { - try (final AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem( + try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem( createConfig("5", "3", "2")))) { assumeBlobServiceType(); AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); @@ -2506,7 +2506,7 @@ public void eTagChangedDuringRename() throws Exception { fs.create(new Path(path, "file1.txt")); fs.create(new Path(path, "file2.txt")); AtomicInteger numberOfCopyBlobCalls = new AtomicInteger(0); - Mockito.doAnswer( copyBlob -> { + Mockito.doAnswer(copyBlob -> { numberOfCopyBlobCalls.incrementAndGet(); return copyBlob.callRealMethod(); }) @@ -2516,7 +2516,7 @@ public void eTagChangedDuringRename() throws Exception { Mockito.any(TracingContext.class)); AtomicInteger numberOfRedoRenameAtomicityCalls = new AtomicInteger(0); - Mockito.doAnswer( redoRenameAtomicity -> { + Mockito.doAnswer(redoRenameAtomicity -> { numberOfRedoRenameAtomicityCalls.incrementAndGet(); return redoRenameAtomicity.callRealMethod(); }) From 47eccd5f533f62f524235e42deb368bcabfe39f6 Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Wed, 19 Feb 2025 22:26:14 -0800 Subject: [PATCH 4/6] Check on backslash on isKeyForDirectorySet method --- .../org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java | 2 +- .../org/apache/hadoop/fs/azurebfs/utils/UriUtils.java | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index 6d2212a17b2c4..e0cb36201065d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -415,7 +415,7 @@ public class AbfsConfiguration{ private int producerQueueMaxSize; @IntegerConfigurationValidatorAnnotation(ConfigurationKey = - FS_AZURE_CONSUMER_MAX_LAG, DefaultValue = DEFAULT_FS_AZURE_CONSUMER_MAX_LAG) + FS_AZURE_CONSUMER_MAX_LAG, DefaultValue = DEFAULT_FS_AZURE_CONSUMER_MAX_LAG) private int listingMaxConsumptionLag; @IntegerConfigurationValidatorAnnotation(ConfigurationKey = diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/UriUtils.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/UriUtils.java index e77336925f6fc..beb0d2d35f89a 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/UriUtils.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/UriUtils.java @@ -248,8 +248,13 @@ private static String replacedUrl(String baseUrl, String oldString, String newSt */ public static boolean isKeyForDirectorySet(String key, Set dirSet) { for (String dir : dirSet) { - if (dir.isEmpty() || key.startsWith( - dir + AbfsHttpConstants.FORWARD_SLASH)) { + // Ensure the directory ends with a forward slash + if (StringUtils.isNotEmpty(dir) + && !dir.endsWith(AbfsHttpConstants.FORWARD_SLASH)) { + dir += AbfsHttpConstants.FORWARD_SLASH; + } + // Return true if the directory is empty or the key starts with the directory + if (dir.isEmpty() || key.startsWith(dir)) { return true; } From 7c71345e4cdc3645007238aaeabf24b43740b80a Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Mon, 3 Mar 2025 02:38:32 -0800 Subject: [PATCH 5/6] Code reformat --- .../hadoop/fs/azurebfs/AbfsCountersImpl.java | 4 +-- .../hadoop/fs/azurebfs/AbfsStatistic.java | 4 +-- .../fs/azurebfs/AzureBlobFileSystem.java | 2 ++ .../fs/azurebfs/services/AbfsBlobClient.java | 3 -- .../fs/azurebfs/services/AbfsClient.java | 32 ------------------- .../fs/azurebfs/services/AbfsDfsClient.java | 5 --- .../azurebfs/services/BlobDeleteHandler.java | 1 - .../azurebfs/services/BlobRenameHandler.java | 4 +-- .../fs/azurebfs/services/RenameAtomicity.java | 4 --- 9 files changed, 5 insertions(+), 54 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java index 9c5b13ce2472e..fdcbc2275ff48 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsCountersImpl.java @@ -54,7 +54,6 @@ import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_CREATED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.DIRECTORIES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.ERROR_IGNORED; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_COPIED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_CREATED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.GET_RESPONSES; @@ -135,8 +134,7 @@ public class AbfsCountersImpl implements AbfsCounters { SERVER_UNAVAILABLE, RENAME_RECOVERY, METADATA_INCOMPLETE_RENAME_FAILURES, - RENAME_PATH_ATTEMPTS, - FILES_COPIED + RENAME_PATH_ATTEMPTS }; private static final AbfsStatistic[] DURATION_TRACKER_LIST = { diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java index bf2a4c8679e9c..3a77e82ffb4fb 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java @@ -109,9 +109,7 @@ public enum AbfsStatistic { "Number of times rename operation failed due to metadata being " + "incomplete"), RENAME_PATH_ATTEMPTS("rename_path_attempts", - "Number of times we attempt to rename a path internally"), - FILES_COPIED("files_copied", - "Total number of files copied from the object store."); + "Number of times we attempt to rename a path internally"); private String statName; private String statDescription; 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 30b9d315ba902..e45603c278d25 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 @@ -993,6 +993,8 @@ public Void call() throws Exception { delete(fs.getPath(), fs.isDirectory()); if (fs.isDirectory()) { statIncrement(DIRECTORIES_DELETED); + } else { + statIncrement(FILES_DELETED); } return null; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java index 33f518d3f70ca..a44d4e5cc843d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java @@ -443,7 +443,6 @@ public void createNonRecursivePreCheck(Path parentPath, } getPathStatus(parentPath.toUri().getPath(), false, tracingContext, null); - incrementAbfsGetPathStatus(); } catch (AbfsRestOperationException ex) { if (ex.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) { throw new FileNotFoundException("Cannot create file " @@ -508,7 +507,6 @@ protected AbfsRestOperation createMarkerAtPath(final String path, final String eTag, final ContextEncryptionAdapter contextEncryptionAdapter, final TracingContext tracingContext) throws AzureBlobFileSystemException { - incrementAbfsCreateFile(); return createPathRestOp(path, false, false, false, eTag, contextEncryptionAdapter, tracingContext); } @@ -1745,7 +1743,6 @@ public void takeGetPathStatusAtomicRenameKeyAction(final Path path, pendingJsonFileStatus = getPathStatus( pendingJsonPath.toUri().getPath(), tracingContext, null, false); - incrementAbfsGetPathStatus(); if (checkIsDir(pendingJsonFileStatus.getResult())) { return; } 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 ded22ff250d9b..58366941331c7 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 @@ -91,10 +91,6 @@ import org.apache.hadoop.util.concurrent.HadoopExecutors; import static org.apache.commons.lang3.StringUtils.isNotEmpty; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_COPIED; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_CREATED; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.FILES_DELETED; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.RENAME_PATH_ATTEMPTS; import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APN_VERSION; @@ -702,34 +698,6 @@ protected void incrementAbfsRenamePath() { abfsCounters.incrementCounter(RENAME_PATH_ATTEMPTS, 1); } - /** - * Increments AbfsCounters for get path status by 1. - */ - protected void incrementAbfsGetPathStatus() { - abfsCounters.incrementCounter(CALL_GET_FILE_STATUS, 1); - } - - /** - * Increments AbfsCounters for Delete File by 1. - */ - protected void incrementAbfsDeleteFile() { - abfsCounters.incrementCounter(FILES_DELETED, 1); - } - - /** - * Increments AbfsCounters for Create File by 1. - */ - protected void incrementAbfsCreateFile() { - abfsCounters.incrementCounter(FILES_CREATED, 1); - } - - /** - * Increments AbfsCounters for Copy Files by 1. - */ - protected void incrementAbfsCopyFile() { - abfsCounters.incrementCounter(FILES_COPIED, 1); - } - /** * Check if the rename request failure is post a retry and if earlier rename * request might have succeeded at back-end. diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java index 4f6d17d578663..952dc556d9e77 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java @@ -70,7 +70,6 @@ import org.apache.hadoop.util.StringUtils; import static org.apache.commons.lang3.StringUtils.isEmpty; -import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ACQUIRE_LEASE_ACTION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APPEND_ACTION; @@ -462,8 +461,6 @@ public void createNonRecursivePreCheck(Path parentPath, + " because parent folder does not exist."); } throw ex; - } finally { - getAbfsCounters().incrementCounter(CALL_GET_FILE_STATUS, 1); } } @@ -1228,8 +1225,6 @@ public AbfsRestOperation deletePath(final String path, } else { return idempotencyOp; } - } finally { - incrementAbfsDeleteFile(); } return op; diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java index 4963b288d3438..d93bafb676cb3 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobDeleteHandler.java @@ -90,7 +90,6 @@ int getMaxConsumptionParallelism() { private boolean deleteInternal(final Path path) throws AzureBlobFileSystemException { getAbfsClient().deleteBlobPath(path, null, tracingContext); - getAbfsClient().incrementAbfsDeleteFile(); deleteCount.incrementAndGet(); return true; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java index 88f736bbb86d7..7daec3d1b7ecd 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java @@ -120,7 +120,6 @@ int getMaxConsumptionParallelism() { * @throws AzureBlobFileSystemException if server call fails */ public boolean execute() throws AzureBlobFileSystemException { - getAbfsClient().incrementAbfsRenamePath(); PathInformation pathInformation = getPathInformation(src, tracingContext); boolean result = false; if (preCheck(src, dst, pathInformation)) { @@ -167,6 +166,7 @@ public boolean execute() throws AzureBlobFileSystemException { result = renameInternal(src, dst); } } finally { + getAbfsClient().incrementAbfsRenamePath(); if (srcAbfsLease != null) { // If the operation is successful, cancel the timer and no need to release // the lease as rename on the blob-path has taken place. @@ -417,9 +417,7 @@ private boolean renameInternal(final Path path, boolean operated = false; try { copyPath(path, destinationPathForBlobPartOfRenameSrcDir, leaseId); - getAbfsClient().incrementAbfsCopyFile(); getAbfsClient().deleteBlobPath(path, leaseId, tracingContext); - getAbfsClient().incrementAbfsDeleteFile(); operated = true; } finally { if (abfsLease != null) { diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java index 72e1c0d379a62..f8dab188f37eb 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/RenameAtomicity.java @@ -267,8 +267,6 @@ public int preRename() throws AzureBlobFileSystemException { } } throw e; - } finally { - abfsClient.incrementAbfsCreateFile(); } } @@ -312,8 +310,6 @@ private void deleteRenamePendingJson() throws AzureBlobFileSystemException { return; } throw e; - } finally { - abfsClient.incrementAbfsDeleteFile(); } } From da81cc8aa43136a11e55781e70449b6864591275 Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Tue, 4 Mar 2025 09:37:38 -0800 Subject: [PATCH 6/6] Changes as per comments given --- .../fs/azurebfs/services/AbfsBlobClient.java | 45 +++++++----- .../fs/azurebfs/services/AbfsDfsClient.java | 3 + .../azurebfs/services/BlobRenameHandler.java | 1 - .../ITestAzureBlobFileSystemRename.java | 73 +++++++++++-------- 4 files changed, 72 insertions(+), 50 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java index a44d4e5cc843d..f184ef5f7a9b7 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java @@ -83,6 +83,7 @@ import static java.net.HttpURLConnection.HTTP_NOT_FOUND; import static java.net.HttpURLConnection.HTTP_OK; import static java.net.HttpURLConnection.HTTP_PRECON_FAILED; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ACQUIRE_LEASE_ACTION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.AND_MARK; @@ -441,8 +442,12 @@ public void createNonRecursivePreCheck(Path parentPath, if (isAtomicRenameKey(parentPath.toUri().getPath())) { takeGetPathStatusAtomicRenameKeyAction(parentPath, tracingContext); } - getPathStatus(parentPath.toUri().getPath(), false, - tracingContext, null); + try { + getPathStatus(parentPath.toUri().getPath(), false, + tracingContext, null); + } finally { + getAbfsCounters().incrementCounter(CALL_GET_FILE_STATUS, 1); + } } catch (AbfsRestOperationException ex) { if (ex.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) { throw new FileNotFoundException("Cannot create file " @@ -804,22 +809,26 @@ public AbfsClientRenameResult renamePath(final String source, BlobRenameHandler blobRenameHandler = getBlobRenameHandler(source, destination, sourceEtag, isAtomicRenameKey(source), tracingContext ); - if (blobRenameHandler.execute()) { - final AbfsUriQueryBuilder abfsUriQueryBuilder - = createDefaultUriQueryBuilder(); - final URL url = createRequestUrl(destination, - abfsUriQueryBuilder.toString()); - final List requestHeaders = createDefaultHeaders(); - final AbfsRestOperation successOp = getSuccessOp( - AbfsRestOperationType.RenamePath, HTTP_METHOD_PUT, - url, requestHeaders); - return new AbfsClientRenameResult(successOp, true, false); - } else { - throw new AbfsRestOperationException(HTTP_INTERNAL_ERROR, - AzureServiceErrorCode.UNKNOWN.getErrorCode(), - ERR_RENAME_BLOB + source + SINGLE_WHITE_SPACE + AND_MARK - + SINGLE_WHITE_SPACE + destination, - null); + try { + if (blobRenameHandler.execute()) { + final AbfsUriQueryBuilder abfsUriQueryBuilder + = createDefaultUriQueryBuilder(); + final URL url = createRequestUrl(destination, + abfsUriQueryBuilder.toString()); + final List requestHeaders = createDefaultHeaders(); + final AbfsRestOperation successOp = getSuccessOp( + AbfsRestOperationType.RenamePath, HTTP_METHOD_PUT, + url, requestHeaders); + return new AbfsClientRenameResult(successOp, true, false); + } else { + throw new AbfsRestOperationException(HTTP_INTERNAL_ERROR, + AzureServiceErrorCode.UNKNOWN.getErrorCode(), + ERR_RENAME_BLOB + source + SINGLE_WHITE_SPACE + AND_MARK + + SINGLE_WHITE_SPACE + destination, + null); + } + } finally { + incrementAbfsRenamePath(); } } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java index 952dc556d9e77..05acaa78f489f 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java @@ -70,6 +70,7 @@ import org.apache.hadoop.util.StringUtils; import static org.apache.commons.lang3.StringUtils.isEmpty; +import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CALL_GET_FILE_STATUS; import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ACQUIRE_LEASE_ACTION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APPEND_ACTION; @@ -461,6 +462,8 @@ public void createNonRecursivePreCheck(Path parentPath, + " because parent folder does not exist."); } throw ex; + } finally { + getAbfsCounters().incrementCounter(CALL_GET_FILE_STATUS, 1); } } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java index 7daec3d1b7ecd..f78228bfcff01 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobRenameHandler.java @@ -166,7 +166,6 @@ public boolean execute() throws AzureBlobFileSystemException { result = renameInternal(src, dst); } } finally { - getAbfsClient().incrementAbfsRenamePath(); if (srcAbfsLease != null) { // If the operation is successful, cancel the timer and no need to release // the lease as rename on the blob-path has taken place. diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java index d239473cca64c..ad352eef69b81 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java @@ -327,12 +327,11 @@ public void testRenameNotFoundBlobToEmptyRoot() throws Exception { @Test public void testRenameBlobToDstWithColonInSourcePath() throws Exception { AzureBlobFileSystem fs = getFileSystem(); - assumeBlobServiceType(); fs.create(new Path("/src:/file")); Assertions.assertThat( - fs.rename(new Path("/src:"), - new Path("/dst")) - ).isTrue(); + fs.rename(new Path("/src:"), new Path("/dst"))) + .describedAs("Rename should succeed") + .isTrue(); } /** @@ -349,9 +348,9 @@ public void testRenameWithColonInDestinationPath() throws Exception { AzureBlobFileSystem fs = getFileSystem(); fs.create(new Path("/src")); Assertions.assertThat( - fs.rename(new Path("/src"), - new Path("/dst:")) - ).isTrue(); + fs.rename(new Path("/src"), new Path("/dst:"))) + .describedAs("Rename should succeed") + .isTrue(); } @Test @@ -364,26 +363,29 @@ public void testRenameWithColonInSourcePath() throws Exception { fs.create(new Path(sourceDirectory + "/Test:", fileName)); // Rename from source to destination Assertions.assertThat( - fs.rename(new Path(sourceDirectory), - new Path(destinationDirectory)) - ).isTrue(); + fs.rename(new Path(sourceDirectory), new Path(destinationDirectory))) + .describedAs("Rename should succeed") + .isTrue(); Assertions.assertThat( - fs.exists(new Path(sourceDirectory, fileName))) + fs.exists(new Path(sourceDirectory, fileName))) + .describedAs("Source directory should not exist after rename") .isFalse(); Assertions.assertThat( fs.exists(new Path(destinationDirectory, fileName))) + .describedAs("Destination directory should exist after rename") .isTrue(); // Rename from destination to source Assertions.assertThat( - fs.rename(new Path(destinationDirectory), - new Path(sourceDirectory)) - ).isTrue(); + fs.rename(new Path(destinationDirectory), new Path(sourceDirectory))) + .describedAs("Rename should succeed").isTrue(); Assertions.assertThat( fs.exists(new Path(sourceDirectory, fileName))) + .describedAs("Destination directory should exist after rename") .isTrue(); Assertions.assertThat( fs.exists(new Path(destinationDirectory, fileName))) + .describedAs("Source directory should not exist after rename") .isFalse(); } @@ -1793,7 +1795,7 @@ private void validateRename(AzureBlobFileSystem fs, Path src, Path dst, * Test the renaming of a directory with different parallelism configurations. */ @Test - public void testRenameDirWithDifferentParallelism() throws Exception { + public void testRenameDirWithDifferentParallelismConfig() throws Exception { try (AzureBlobFileSystem currentFs = getFileSystem()) { assumeBlobServiceType(); Path src = new Path("/hbase/A1/A2"); @@ -2073,7 +2075,8 @@ private void validateAtomicRenameKey(AbfsBlobClient abfsBlobClient, String path, * @return file system * @throws IOException in case of failure */ - public AzureBlobFileSystem createJsonFile(Path path, Path renameJson) throws IOException { + public AzureBlobFileSystem createJsonFile(Path path, Path renameJson) + throws IOException { final AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem()); assumeBlobServiceType(); AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore()); @@ -2084,9 +2087,12 @@ public AzureBlobFileSystem createJsonFile(Path path, Path renameJson) throws IOE fs.setWorkingDirectory(new Path(ROOT_PATH)); fs.create(new Path(path, "file.txt")); - AzureBlobFileSystemStore.VersionedFileStatus fileStatus = (AzureBlobFileSystemStore.VersionedFileStatus) fs.getFileStatus(path); + AzureBlobFileSystemStore.VersionedFileStatus fileStatus + = (AzureBlobFileSystemStore.VersionedFileStatus) fs.getFileStatus(path); - new RenameAtomicity(path, new Path("/hbase/test4"), renameJson, getTestTracingContext(fs, true), fileStatus.getEtag(), client) + new RenameAtomicity(path, new Path("/hbase/test4"), + renameJson, getTestTracingContext(fs, true), + fileStatus.getEtag(), client) .preRename(); Assertions.assertThat(fs.exists(renameJson)) @@ -2106,7 +2112,7 @@ public AzureBlobFileSystem createJsonFile(Path path, Path renameJson) throws IOE * @throws Exception if any error occurs during the test execution */ @Test - public void listCrashRecoveryWithSingleChildFolder() throws Exception { + public void testListCrashRecoveryWithSingleChildFolder() throws Exception { AzureBlobFileSystem fs = null; try { Path path = new Path("/hbase/A1/A2"); @@ -2135,7 +2141,7 @@ public void listCrashRecoveryWithSingleChildFolder() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void listCrashRecoveryWithMultipleChildFolder() throws Exception { + public void testListCrashRecoveryWithMultipleChildFolder() throws Exception { AzureBlobFileSystem fs = null; try { Path path = new Path("/hbase/A1/A2"); @@ -2167,7 +2173,7 @@ public void listCrashRecoveryWithMultipleChildFolder() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void listCrashRecoveryWithPendingJsonFile() throws Exception { + public void testListCrashRecoveryWithPendingJsonFile() throws Exception { AzureBlobFileSystem fs = null; try { Path path = new Path("/hbase/A1/A2"); @@ -2200,7 +2206,7 @@ public void listCrashRecoveryWithPendingJsonFile() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void listCrashRecoveryWithoutAnyPendingJsonFile() throws Exception { + public void testListCrashRecoveryWithoutAnyPendingJsonFile() throws Exception { AzureBlobFileSystem fs = null; try { Path path = new Path("/hbase/A1/A2"); @@ -2233,7 +2239,7 @@ public void listCrashRecoveryWithoutAnyPendingJsonFile() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void listCrashRecoveryWithPendingJsonDir() throws Exception { + public void testListCrashRecoveryWithPendingJsonDir() throws Exception { try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem())) { assumeBlobServiceType(); AbfsBlobClient client = (AbfsBlobClient) addSpyHooksOnClient(fs); @@ -2280,7 +2286,7 @@ public void listCrashRecoveryWithPendingJsonDir() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void listCrashRecoveryWithMultipleJsonFile() throws Exception { + public void testListCrashRecoveryWithMultipleJsonFile() throws Exception { AzureBlobFileSystem fs = null; try { Path path = new Path("/hbase/A1/A2"); @@ -2295,9 +2301,12 @@ public void listCrashRecoveryWithMultipleJsonFile() throws Exception { fs.create(new Path(path2, "file3.txt")); Path renameJson2 = new Path(path2.getParent(), path2.getName() + SUFFIX); - AzureBlobFileSystemStore.VersionedFileStatus fileStatus = (AzureBlobFileSystemStore.VersionedFileStatus) fs.getFileStatus(path2); + AzureBlobFileSystemStore.VersionedFileStatus fileStatus + = (AzureBlobFileSystemStore.VersionedFileStatus) fs.getFileStatus(path2); - new RenameAtomicity(path2, new Path("/hbase/test4"), renameJson2, getTestTracingContext(fs, true), fileStatus.getEtag(), client).preRename(); + new RenameAtomicity(path2, new Path("/hbase/test4"), + renameJson2, getTestTracingContext(fs, true), + fileStatus.getEtag(), client).preRename(); fs.create(new Path(path, "file2.txt")); @@ -2334,7 +2343,7 @@ public void listCrashRecoveryWithMultipleJsonFile() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void getPathStatusWithPendingJsonFile() throws Exception { + public void testGetPathStatusWithPendingJsonFile() throws Exception { AzureBlobFileSystem fs = null; try { Path path = new Path("/hbase/A1/A2"); @@ -2387,7 +2396,7 @@ public void getPathStatusWithPendingJsonFile() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void getPathStatusWithoutPendingJsonFile() throws Exception { + public void testGetPathStatusWithoutPendingJsonFile() throws Exception { try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem())) { assumeBlobServiceType(); @@ -2441,7 +2450,7 @@ public void getPathStatusWithoutPendingJsonFile() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void getPathStatusWithPendingJsonDir() throws Exception { + public void testGetPathStatusWithPendingJsonDir() throws Exception { try (AzureBlobFileSystem fs = Mockito.spy(this.getFileSystem())) { assumeBlobServiceType(); @@ -2466,7 +2475,9 @@ public void getPathStatusWithPendingJsonDir() throws Exception { conf.getClientCorrelationId(), fs.getFileSystemId(), FSOperationType.GET_FILESTATUS, TracingHeaderFormat.ALL_ID_FORMAT, null); - AbfsHttpOperation abfsHttpOperation = client.getPathStatus(path.toUri().getPath(), true, tracingContext, null).getResult(); + AbfsHttpOperation abfsHttpOperation + = client.getPathStatus(path.toUri().getPath(), true, + tracingContext, null).getResult(); Assertions.assertThat(abfsHttpOperation.getStatusCode()) .describedAs("Path should be found.") @@ -2502,7 +2513,7 @@ public void getPathStatusWithPendingJsonDir() throws Exception { * @throws Exception if any error occurs during the test execution */ @Test - public void eTagChangedDuringRename() throws Exception { + public void testETagChangedDuringRename() throws Exception { AzureBlobFileSystem fs = null; try { assumeBlobServiceType();