You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/06/15 12:30:50 UTC
[GitHub] [hadoop] 9uapaw commented on a diff in pull request #4439: YARN-11182. Refactor TestAggregatedLogDeletionService: 2nd phase
9uapaw commented on code in PR #4439:
URL: https://github.com/apache/hadoop/pull/4439#discussion_r897915514
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogDeletionService.java:
##########
@@ -138,82 +97,35 @@ public void testDeletion() throws Exception {
long toKeepTime = now - (1500 * 1000);
Configuration conf = setupConfiguration(1800, -1);
-
- Path rootPath = new Path(ROOT);
- FileSystem rootFs = rootPath.getFileSystem(conf);
- FileSystem mockFs = ((FilterFileSystem)rootFs).getRawFileSystem();
-
- Path remoteRootLogPath = new Path(REMOTE_ROOT_LOG_DIR);
- PathWithFileStatus userDir = createDirLogPathWithFileStatus(remoteRootLogPath, USER_ME,
- toKeepTime);
-
- when(mockFs.listStatus(remoteRootLogPath)).thenReturn(new FileStatus[]{userDir.fileStatus});
-
- ApplicationId appId1 = ApplicationId.newInstance(now, 1);
- ApplicationId appId2 = ApplicationId.newInstance(now, 2);
- ApplicationId appId3 = ApplicationId.newInstance(now, 3);
- ApplicationId appId4 = ApplicationId.newInstance(now, 4);
-
- PathWithFileStatus suffixDir = createDirLogPathWithFileStatus(userDir.path, NEW_SUFFIX,
- toDeleteTime);
- PathWithFileStatus bucketDir = createDirLogPathWithFileStatus(remoteRootLogPath, SUFFIX,
- toDeleteTime);
-
- PathWithFileStatus app1 = createPathWithFileStatusForAppId(remoteRootLogPath, appId1,
- USER_ME, SUFFIX, toDeleteTime);
- PathWithFileStatus app2 = createPathWithFileStatusForAppId(remoteRootLogPath, appId2,
- USER_ME, SUFFIX, toDeleteTime);
- PathWithFileStatus app3 = createPathWithFileStatusForAppId(remoteRootLogPath, appId3,
- USER_ME, SUFFIX, toDeleteTime);
- PathWithFileStatus app4 = createPathWithFileStatusForAppId(remoteRootLogPath, appId4,
- USER_ME, SUFFIX, toDeleteTime);
-
- when(mockFs.listStatus(userDir.path)).thenReturn(new FileStatus[] {suffixDir.fileStatus});
- when(mockFs.listStatus(suffixDir.path)).thenReturn(new FileStatus[] {bucketDir.fileStatus});
- when(mockFs.listStatus(bucketDir.path)).thenReturn(new FileStatus[] {
- app1.fileStatus, app2.fileStatus, app3.fileStatus, app4.fileStatus});
-
- PathWithFileStatus app2Log1 = createFileLogPathWithFileStatus(app2.path, DIR_HOST1,
- toDeleteTime);
- PathWithFileStatus app2Log2 = createFileLogPathWithFileStatus(app2.path, DIR_HOST2, toKeepTime);
- PathWithFileStatus app3Log1 = createFileLogPathWithFileStatus(app3.path, DIR_HOST1,
- toDeleteTime);
- PathWithFileStatus app3Log2 = createFileLogPathWithFileStatus(app3.path, DIR_HOST2,
- toDeleteTime);
- PathWithFileStatus app4Log1 = createFileLogPathWithFileStatus(app4.path, DIR_HOST1,
- toDeleteTime);
- PathWithFileStatus app4Log2 = createFileLogPathWithFileStatus(app4.path, DIR_HOST2, toKeepTime);
-
- when(mockFs.listStatus(app1.path)).thenReturn(new FileStatus[]{});
- when(mockFs.listStatus(app2.path)).thenReturn(new FileStatus[]{app2Log1.fileStatus,
- app2Log2.fileStatus});
- when(mockFs.listStatus(app3.path)).thenReturn(new FileStatus[]{app3Log1.fileStatus,
- app3Log2.fileStatus});
- when(mockFs.listStatus(app4.path)).thenReturn(new FileStatus[]{app4Log1.fileStatus,
- app4Log2.fileStatus});
- when(mockFs.delete(app3.path, true)).thenThrow(
- new AccessControlException("Injected Error\nStack Trace :("));
-
- final List<ApplicationId> finishedApplications = Collections.unmodifiableList(
- Arrays.asList(appId1, appId2, appId3));
- final List<ApplicationId> runningApplications = Collections.singletonList(appId4);
-
- AggregatedLogDeletionService deletionService =
- new AggregatedLogDeletionServiceForTest(runningApplications, finishedApplications);
- deletionService.init(conf);
- deletionService.start();
-
- int timeout = 2000;
- verify(mockFs, timeout(timeout)).delete(app1.path, true);
- verify(mockFs, timeout(timeout).times(0)).delete(app2.path, true);
- verify(mockFs, timeout(timeout)).delete(app3.path, true);
- verify(mockFs, timeout(timeout).times(0)).delete(app4.path, true);
- verify(mockFs, timeout(timeout)).delete(app4Log1.path, true);
- verify(mockFs, timeout(timeout).times(0)).delete(app4Log2.path, true);
-
- deletionService.stop();
+ long timeout = 2000L;
+ LogAggregationFilesBuilder.create(conf)
+ .withRootPath(ROOT)
+ .withRemoteRootLogPath(REMOTE_ROOT_LOG_DIR)
+ .withUserDir(USER_ME, toKeepTime)
+ .withSuffixDir(SUFFIX, toDeleteTime)
+ .withBucketDir(toDeleteTime)
+ .withApps(new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime, new Pair[] {}),
+ new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime,
+ Pair.of(DIR_HOST1, toDeleteTime),
+ Pair.of(DIR_HOST2, toKeepTime)),
+ new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime,
+ Pair.of(DIR_HOST1, toDeleteTime),
+ Pair.of(DIR_HOST2, toDeleteTime)),
+ new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime,
+ Pair.of(DIR_HOST1, toDeleteTime),
+ Pair.of(DIR_HOST2, toKeepTime)))
+ .withFinishedApps(1, 2, 3)
+ .withRunningApps(4)
+ .injectExceptionForAppDirDeletion(3)
+ .setupMocks()
+ .setupAndRunDeletionService()
+ .verifyAppDirsDeleted(timeout, 1, 3)
+ .verifyAppDirsNotDeleted(timeout, 2, 4)
+ .verifyAppFileDeleted(4, 1, timeout)
+ .verifyAppFileNotDeleted(4, 2, timeout)
+ .teardown();
Review Comment:
I think verifying and running operations should not be the responsibility of a builder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org