diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/SharedCacheUploader.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/SharedCacheUploader.java index 23aa5b3..b7b9396 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/SharedCacheUploader.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/SharedCacheUploader.java @@ -138,6 +138,7 @@ public Boolean call() throws Exception { tempPath = new Path(directoryPath, getTemporaryFileName(actualPath)); if (!uploadFile(actualPath, tempPath)) { LOG.warn("Could not copy the file to the shared cache at " + tempPath); + deleteTempFile(tempPath); return false; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/TestSharedCacheUploader.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/TestSharedCacheUploader.java index 9234c62..7ea2ea6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/TestSharedCacheUploader.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/sharedcache/TestSharedCacheUploader.java @@ -46,7 +46,7 @@ public class TestSharedCacheUploader { /** - * If verifyAccess fails, the upload should fail + * If verifyAccess fails, the upload should fail. */ @Test public void testFailVerifyAccess() throws Exception { @@ -57,7 +57,7 @@ public void testFailVerifyAccess() throws Exception { } /** - * If rename fails, the upload should fail + * If rename fails, the upload should fail. */ @Test public void testRenameFail() throws Exception { @@ -92,8 +92,43 @@ public void testRenameFail() throws Exception { } /** + * If uploadFile fails, the upload should fail. + */ + @Test + public void testUploadFileFail() throws Exception { + Configuration conf = new Configuration(); + conf.setBoolean(YarnConfiguration.SHARED_CACHE_ENABLED, true); + LocalResource resource = mock(LocalResource.class); + Path localPath = mock(Path.class); + when(localPath.getName()).thenReturn("foo.jar"); + String user = "joe"; + SCMUploaderProtocol scmClient = mock(SCMUploaderProtocol.class); + SCMUploaderNotifyResponse response = mock(SCMUploaderNotifyResponse.class); + when(response.getAccepted()).thenReturn(true); + when(scmClient.notify(isA(SCMUploaderNotifyRequest.class))). + thenReturn(response); + FileSystem fs = mock(FileSystem.class); + // return true when rename is called + when(fs.rename(isA(Path.class), isA(Path.class))).thenReturn(true); + FileSystem localFs = FileSystem.getLocal(conf); + SharedCacheUploader spied = + createSpiedUploader(resource, localPath, user, conf, scmClient, fs, + localFs); + // stub verifyAccess() to return true + doReturn(true).when(spied).verifyAccess(); + // stub getActualPath() + doReturn(localPath).when(spied).getActualPath(); + // stub computeChecksum() + doReturn("abcdef0123456789").when(spied).computeChecksum(isA(Path.class)); + // stub uploadFile() to return true + doReturn(false).when(spied).uploadFile(isA(Path.class), isA(Path.class)); + + assertFalse(spied.call()); + } + + /** * If verifyAccess, uploadFile, rename, and notification succeed, the upload - * should succeed + * should succeed. */ @Test public void testSuccess() throws Exception { @@ -132,7 +167,7 @@ public void testSuccess() throws Exception { /** * If verifyAccess, uploadFile, and rename succed, but it receives a nay from - * SCM, the file should be deleted + * SCM, the file should be deleted. */ @Test public void testNotifySCMFail() throws Exception { @@ -166,7 +201,7 @@ public void testNotifySCMFail() throws Exception { } /** - * If resource is public, verifyAccess should succeed + * If resource is public, verifyAccess should succeed. */ @Test public void testVerifyAccessPublicResource() throws Exception { @@ -190,7 +225,7 @@ public void testVerifyAccessPublicResource() throws Exception { /** * If the localPath does not exists, getActualPath should get to one level - * down + * down. */ @Test public void testGetActualPath() throws Exception {