Skip to content

Commit a7371a7

Browse files
MAPREDUCE-7225: Fix broken current folder expansion during MR job start. Contributed by Peter Bacsko.
1 parent 89b102f commit a7371a7

File tree

2 files changed

+85
-4
lines changed

2 files changed

+85
-4
lines changed

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/JobResourceUploader.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
class JobResourceUploader {
6060
protected static final Logger LOG =
6161
LoggerFactory.getLogger(JobResourceUploader.class);
62+
private static final String ROOT_PATH = "/";
63+
6264
private final boolean useWildcard;
6365
private final FileSystem jtFs;
6466
private SharedCacheClient scClient = null;
@@ -674,9 +676,30 @@ Path copyRemoteFiles(Path parentDir, Path originalPath,
674676
if (FileUtil.compareFs(remoteFs, jtFs)) {
675677
return originalPath;
676678
}
679+
680+
boolean root = false;
681+
if (ROOT_PATH.equals(originalPath.toUri().getPath())) {
682+
// "/" needs special treatment
683+
root = true;
684+
} else {
685+
// If originalPath ends in a "/", then remove it so
686+
// that originalPath.getName() does not return an empty string
687+
String uriString = originalPath.toUri().toString();
688+
if (uriString.endsWith("/")) {
689+
try {
690+
URI strippedURI =
691+
new URI(uriString.substring(0, uriString.length() - 1));
692+
originalPath = new Path(strippedURI);
693+
} catch (URISyntaxException e) {
694+
throw new IllegalArgumentException("Error processing URI", e);
695+
}
696+
}
697+
}
698+
677699
// this might have name collisions. copy will throw an exception
678700
// parse the original path to create new path
679-
Path newPath = new Path(parentDir, originalPath.getName());
701+
Path newPath = root ?
702+
parentDir : new Path(parentDir, originalPath.getName());
680703
FileUtil.copy(remoteFs, originalPath, jtFs, newPath, false, conf);
681704
jtFs.setReplication(newPath, replication);
682705
jtFs.makeQualified(newPath);

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestJobResourceUploader.java

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,19 @@
1818

1919
package org.apache.hadoop.mapreduce;
2020

21+
import static org.mockito.Mockito.doReturn;
2122
import static org.mockito.Mockito.mock;
2223
import static org.mockito.Mockito.when;
2324
import static org.mockito.Mockito.never;
2425
import static org.mockito.Mockito.verify;
2526
import static org.mockito.Mockito.times;
27+
import static org.mockito.Mockito.spy;
2628
import static org.mockito.ArgumentMatchers.any;
2729
import static org.mockito.ArgumentMatchers.eq;
2830

2931
import java.io.IOException;
3032
import java.net.URI;
33+
import java.net.URISyntaxException;
3134
import java.util.Arrays;
3235
import java.util.Collection;
3336
import java.util.HashMap;
@@ -46,8 +49,10 @@
4649
import org.apache.hadoop.mapred.JobConf;
4750
import org.junit.Assert;
4851
import org.junit.Test;
52+
import org.mockito.ArgumentCaptor;
4953
import org.mockito.verification.VerificationMode;
5054

55+
5156
/**
5257
* A class for unit testing JobResourceUploader.
5358
*/
@@ -375,6 +380,50 @@ public void testErasureCodingDisabled() throws IOException {
375380
testErasureCodingSetting(false);
376381
}
377382

383+
@Test
384+
public void testOriginalPathEndsInSlash()
385+
throws IOException, URISyntaxException {
386+
testOriginalPathWithTrailingSlash(
387+
new Path(new URI("file:/local/mapred/test/")),
388+
new Path("hdfs://localhost:1234/home/hadoop/test/"));
389+
}
390+
391+
@Test
392+
public void testOriginalPathIsRoot() throws IOException, URISyntaxException {
393+
testOriginalPathWithTrailingSlash(
394+
new Path(new URI("file:/")),
395+
new Path("hdfs://localhost:1234/home/hadoop/"));
396+
}
397+
398+
private void testOriginalPathWithTrailingSlash(Path path,
399+
Path expectedRemotePath) throws IOException, URISyntaxException {
400+
Path dstPath = new Path("hdfs://localhost:1234/home/hadoop/");
401+
DistributedFileSystem fs = mock(DistributedFileSystem.class);
402+
// make sure that FileUtils.copy() doesn't try to copy anything
403+
when(fs.mkdirs(any(Path.class))).thenReturn(false);
404+
when(fs.getUri()).thenReturn(dstPath.toUri());
405+
406+
JobResourceUploader uploader = new StubedUploader(fs, true, true);
407+
JobConf jConf = new JobConf();
408+
Path originalPath = spy(path);
409+
FileSystem localFs = mock(FileSystem.class);
410+
FileStatus fileStatus = mock(FileStatus.class);
411+
when(localFs.getFileStatus(any(Path.class))).thenReturn(fileStatus);
412+
when(fileStatus.isDirectory()).thenReturn(true);
413+
when(fileStatus.getPath()).thenReturn(originalPath);
414+
415+
doReturn(localFs).when(originalPath)
416+
.getFileSystem(any(Configuration.class));
417+
when(localFs.getUri()).thenReturn(path.toUri());
418+
419+
uploader.copyRemoteFiles(dstPath,
420+
originalPath, jConf, (short) 1);
421+
422+
ArgumentCaptor<Path> pathCaptor = ArgumentCaptor.forClass(Path.class);
423+
verify(fs).makeQualified(pathCaptor.capture());
424+
Assert.assertEquals("Path", expectedRemotePath, pathCaptor.getValue());
425+
}
426+
378427
private void testErasureCodingSetting(boolean defaultBehavior)
379428
throws IOException {
380429
JobConf jConf = new JobConf();
@@ -387,7 +436,7 @@ private void testErasureCodingSetting(boolean defaultBehavior)
387436
DistributedFileSystem fs = mock(DistributedFileSystem.class);
388437
Path path = new Path("/");
389438
when(fs.makeQualified(any(Path.class))).thenReturn(path);
390-
JobResourceUploader uploader = new StubedUploader(fs, true);
439+
JobResourceUploader uploader = new StubedUploader(fs, true, false);
391440
Job job = Job.getInstance(jConf);
392441

393442
uploader.uploadResources(job, new Path("/test"));
@@ -728,6 +777,8 @@ private String buildPathStringSub(String pathPrefix, String processedPath,
728777
}
729778

730779
private class StubedUploader extends JobResourceUploader {
780+
private boolean callOriginalCopy = false;
781+
731782
StubedUploader(JobConf conf) throws IOException {
732783
this(conf, false);
733784
}
@@ -736,8 +787,10 @@ private class StubedUploader extends JobResourceUploader {
736787
super(FileSystem.getLocal(conf), useWildcard);
737788
}
738789

739-
StubedUploader(FileSystem fs, boolean useWildcard) throws IOException {
790+
StubedUploader(FileSystem fs, boolean useWildcard,
791+
boolean callOriginalCopy) throws IOException {
740792
super(fs, useWildcard);
793+
this.callOriginalCopy = callOriginalCopy;
741794
}
742795

743796
@Override
@@ -757,7 +810,12 @@ boolean mkdirs(FileSystem fs, Path dir, FsPermission permission)
757810
@Override
758811
Path copyRemoteFiles(Path parentDir, Path originalPath, Configuration conf,
759812
short replication) throws IOException {
760-
return new Path(destinationPathPrefix + originalPath.getName());
813+
if (callOriginalCopy) {
814+
return super.copyRemoteFiles(
815+
parentDir, originalPath, conf, replication);
816+
} else {
817+
return new Path(destinationPathPrefix + originalPath.getName());
818+
}
761819
}
762820

763821
@Override

0 commit comments

Comments
 (0)