Skip to content

Commit af66bd8

Browse files
committed
Addressed comments
1 parent 18f028c commit af66bd8

File tree

4 files changed

+364
-341
lines changed

4 files changed

+364
-341
lines changed

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
import org.apache.hadoop.fs.azurebfs.utils.EncryptionType;
8484
import org.apache.hadoop.fs.azurebfs.utils.MetricFormat;
8585
import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
86+
import org.apache.hadoop.fs.azurebfs.utils.UriUtils;
8687
import org.apache.hadoop.fs.permission.FsAction;
8788
import org.apache.hadoop.fs.permission.FsPermission;
8889
import org.apache.hadoop.fs.store.LogExactlyOnce;
@@ -1536,8 +1537,11 @@ public void getMetricCall(TracingContext tracingContext) throws IOException {
15361537
final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
15371538
abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM);
15381539

1539-
final URL url = createRequestUrl(new URL(abfsMetricUrl), EMPTY_STRING, abfsUriQueryBuilder.toString());
1540-
1540+
// Construct the URL for the metric call
1541+
// In case of blob storage, the URL is changed to DFS URL
1542+
final URL url = UriUtils.changeUrlFromBlobToDfs(
1543+
createRequestUrl(new URL(abfsMetricUrl),
1544+
EMPTY_STRING, abfsUriQueryBuilder.toString()));
15411545
final AbfsRestOperation op = getAbfsRestOperation(
15421546
AbfsRestOperationType.GetFileSystemProperties,
15431547
HTTP_METHOD_HEAD,

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemRename.java

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
import static org.apache.hadoop.fs.azurebfs.services.AbfsClientTestUtil.mockAddClientTransactionIdToHeader;
100100
import static org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_RENAME_RECOVERY;
101101
import static org.apache.hadoop.fs.azurebfs.services.RenameAtomicity.SUFFIX;
102+
import static org.apache.hadoop.fs.azurebfs.utils.AbfsTestUtils.createFiles;
102103
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsFile;
103104
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs;
104105
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
@@ -1760,7 +1761,7 @@ public void testRenameDirWithDifferentParallelismConfig() throws Exception {
17601761
Path dst = new Path("/hbase/A1/A3");
17611762

17621763
// Create sample files in the source directory
1763-
ITestAzureBlobFileSystemRenameRecovery.createFiles(currentFs, src, TOTAL_FILES);
1764+
createFiles(currentFs, src, TOTAL_FILES);
17641765

17651766
// Test renaming with different configurations
17661767
renameDir(currentFs, "10", "5", "2", src, dst);
@@ -2030,7 +2031,7 @@ public void testRenamePathRetryIdempotency() throws Exception {
20302031
Path destFilePath = new Path(sourceDir, "file2");
20312032

20322033
final List<AbfsHttpHeader> headers = new ArrayList<>();
2033-
mockRetriedRequest(abfsClient, headers);
2034+
mockRetriedRequest(abfsClient, headers, 0);
20342035

20352036
AbfsRestOperation getPathRestOp = Mockito.mock(AbfsRestOperation.class);
20362037
AbfsHttpOperation op = Mockito.mock(AbfsHttpOperation.class);
@@ -2054,7 +2055,9 @@ public void testRenamePathRetryIdempotency() throws Exception {
20542055
Mockito.nullable(String.class), Mockito.nullable(Boolean.class),
20552056
Mockito.nullable(TracingContext.class),
20562057
Mockito.nullable(ContextEncryptionAdapter.class));
2057-
fs.rename(sourceFilePath, destFilePath);
2058+
Assertions.assertThat(fs.rename(sourceFilePath, destFilePath))
2059+
.describedAs("Rename should succeed.")
2060+
.isTrue();
20582061
}
20592062
}
20602063

@@ -2114,7 +2117,7 @@ public void testFailureInGetPathStatusDuringRenameRecovery() throws Exception {
21142117
fs.getAbfsStore().setClient(abfsDfsClient);
21152118
final String[] clientTransactionId = new String[1];
21162119
mockAddClientTransactionIdToHeader(abfsDfsClient, clientTransactionId);
2117-
mockRetriedRequest(abfsDfsClient, new ArrayList<>());
2120+
mockRetriedRequest(abfsDfsClient, new ArrayList<>(), 1);
21182121
int[] flag = new int[1];
21192122
Mockito.doAnswer(getPathStatus -> {
21202123
if (flag[0] == 1) {
@@ -2241,7 +2244,7 @@ public void testDirRenameWithDestAsRoot() throws Exception {
22412244
* @throws Exception If an error occurs during mock creation or operation execution.
22422245
*/
22432246
private void mockRetriedRequest(AbfsDfsClient abfsDfsClient,
2244-
final List<AbfsHttpHeader> headers) throws Exception {
2247+
final List<AbfsHttpHeader> headers, int failedCall) throws Exception {
22452248
TestAbfsClient.mockAbfsOperationCreation(abfsDfsClient,
22462249
new MockIntercept<AbfsRestOperation>() {
22472250
private int count = 0;
@@ -2265,6 +2268,97 @@ public void answer(final AbfsRestOperation mockedObj,
22652268
SOURCE_PATH_NOT_FOUND.getErrorCode(), EMPTY_STRING, null, op);
22662269
}
22672270
}
2268-
}, 1);
2271+
}, failedCall);
22692272
}
2273+
2274+
/**
2275+
* Tests the rename operation with multiple directories in the source path.
2276+
* This test verifies that the rename operation correctly handles
2277+
* multiple directories and files, ensuring that the source directory
2278+
* is renamed to the destination path.
2279+
*
2280+
* @throws Exception if an error occurs during the test execution
2281+
*/
2282+
@Test
2283+
public void testRenameWithMultipleDirsInSource() throws Exception {
2284+
try (AzureBlobFileSystem fs = this.getFileSystem()) {
2285+
assumeBlobServiceType();
2286+
fs.mkdirs(new Path("/testDir/dir1"));
2287+
for (int i = 0; i < 10; i++) {
2288+
fs.create(new Path("/testDir/dir1/file" + i));
2289+
}
2290+
fs.mkdirs(new Path("/testDir/dir2"));
2291+
fs.create(new Path("/testDir/dir2/file2"));
2292+
createAzCopyFolder(new Path("/testDir/dir3"));
2293+
Assertions.assertThat(fs.rename(new Path("/testDir"),
2294+
new Path("/testDir2")))
2295+
.describedAs("Rename should succeed.")
2296+
.isTrue();
2297+
Assertions.assertThat(fs.exists(new Path("/testDir")))
2298+
.describedAs("Old directory should not exist.")
2299+
.isFalse();
2300+
Assertions.assertThat(fs.exists(new Path("/testDir2")))
2301+
.describedAs("New directory should exist.")
2302+
.isTrue();
2303+
}
2304+
}
2305+
2306+
/**
2307+
* Tests the rename operation with multiple implicit directories in the source path.
2308+
* This test verifies that the rename operation correctly handles
2309+
* multiple directories and files, ensuring that the source directory
2310+
* is renamed to the destination path.
2311+
*
2312+
* @throws Exception if an error occurs during the test execution
2313+
*/
2314+
@Test
2315+
public void testRenameWithMultipleImplicitDirsInSource() throws Exception {
2316+
try (AzureBlobFileSystem fs = this.getFileSystem()) {
2317+
assumeBlobServiceType();
2318+
createAzCopyFolder(new Path("/testDir/dir1"));
2319+
for (int i = 0; i < 10; i++) {
2320+
createAzCopyFile(new Path("/testDir/dir1/file" + i));
2321+
}
2322+
createAzCopyFolder(new Path("/testDir/dir2"));
2323+
createAzCopyFile(new Path("/testDir/dir2/file2"));
2324+
createAzCopyFolder(new Path("/testDir/dir3"));
2325+
Assertions.assertThat(fs.rename(new Path("/testDir"),
2326+
new Path("/testDir2")))
2327+
.describedAs("Rename should succeed.")
2328+
.isTrue();
2329+
Assertions.assertThat(fs.exists(new Path("/testDir")))
2330+
.describedAs("Old directory should not exist.")
2331+
.isFalse();
2332+
Assertions.assertThat(fs.exists(new Path("/testDir2")))
2333+
.describedAs("New directory should exist.")
2334+
.isTrue();
2335+
}
2336+
}
2337+
2338+
/**
2339+
* Tests renaming a directory with an explicit directory in the source path.
2340+
* This test verifies that the rename operation correctly handles
2341+
* the explicit directory and files, ensuring that the source directory
2342+
* is renamed to the destination path.
2343+
*
2344+
* @throws Exception if an error occurs during the test execution
2345+
*/
2346+
@Test
2347+
public void testRenameWithExplicitDirInSource() throws Exception {
2348+
try (AzureBlobFileSystem fs = this.getFileSystem()) {
2349+
assumeBlobServiceType();
2350+
fs.create(new Path("/testDir/dir3/file2"));
2351+
fs.create(new Path("/testDir/dir3/file1"));
2352+
Assertions.assertThat(fs.rename(new Path("/testDir"),
2353+
new Path("/testDir2")))
2354+
.describedAs("Rename should succeed.")
2355+
.isTrue();
2356+
Assertions.assertThat(fs.exists(new Path("/testDir")))
2357+
.describedAs("Old directory should not exist.")
2358+
.isFalse();
2359+
Assertions.assertThat(fs.exists(new Path("/testDir2")))
2360+
.describedAs("New directory should exist.")
2361+
.isTrue();
2362+
}
2363+
}
22702364
}

0 commit comments

Comments
 (0)