[
https://issues.apache.org/jira/browse/YARN-11241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17602753#comment-17602753
]
ASF GitHub Bot commented on YARN-11241:
---------------------------------------
aajisaka commented on code in PR #4703:
URL: https://github.com/apache/hadoop/pull/4703#discussion_r967688449
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java:
##########
@@ -234,31 +234,41 @@ private void verifyLocalFileDeletion(
// ensure filesystems were closed
verify(logAggregationService).closeFileSystems(
any(UserGroupInformation.class));
- List<Path> dirList = new ArrayList<>();
- dirList.add(new Path(app1LogDir.toURI()));
- verify(delSrvc, times(2)).delete(argThat(new FileDeletionMatcher(
- delSrvc, user, null, dirList)));
-
- String containerIdStr = container11.toString();
- File containerLogDir = new File(app1LogDir, containerIdStr);
- int count = 0;
- int maxAttempts = 50;
- for (String fileType : new String[] { "stdout", "stderr", "syslog" }) {
- File f = new File(containerLogDir, fileType);
- count = 0;
- while ((f.exists()) && (count < maxAttempts)) {
- count++;
- Thread.sleep(100);
+ boolean filesShouldBeDeleted =
+
this.conf.getBoolean(YarnConfiguration.LOG_AGGREGATION_ENABLE_LOCAL_CLEANUP,
+ YarnConfiguration.DEFAULT_LOG_AGGREGATION_ENABLE_LOCAL_CLEANUP);
+ if (filesShouldBeDeleted) {
+ List<Path> dirList = new ArrayList<>();
+ dirList.add(new Path(app1LogDir.toURI()));
+ verify(delSrvc, times(2)).delete(argThat(new FileDeletionMatcher(
+ delSrvc, user, null, dirList)));
+
+ String containerIdStr = container11.toString();
+ File containerLogDir = new File(app1LogDir, containerIdStr);
+ for (String fileType : new String[]{"stdout", "stderr", "syslog"}) {
+ File f = new File(containerLogDir, fileType);
+ Thread.sleep(5000);
+ Assert.assertFalse("File [" + f + "] was not deleted", f.exists());
}
Review Comment:
In this case, I recommend to use while loop. That way we can continue the
test as soon as the file is deleted instead of waiting for 5 sec.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java:
##########
@@ -234,31 +234,41 @@ private void verifyLocalFileDeletion(
// ensure filesystems were closed
verify(logAggregationService).closeFileSystems(
any(UserGroupInformation.class));
- List<Path> dirList = new ArrayList<>();
- dirList.add(new Path(app1LogDir.toURI()));
- verify(delSrvc, times(2)).delete(argThat(new FileDeletionMatcher(
- delSrvc, user, null, dirList)));
-
- String containerIdStr = container11.toString();
- File containerLogDir = new File(app1LogDir, containerIdStr);
- int count = 0;
- int maxAttempts = 50;
- for (String fileType : new String[] { "stdout", "stderr", "syslog" }) {
- File f = new File(containerLogDir, fileType);
- count = 0;
- while ((f.exists()) && (count < maxAttempts)) {
- count++;
- Thread.sleep(100);
+ boolean filesShouldBeDeleted =
+
this.conf.getBoolean(YarnConfiguration.LOG_AGGREGATION_ENABLE_LOCAL_CLEANUP,
+ YarnConfiguration.DEFAULT_LOG_AGGREGATION_ENABLE_LOCAL_CLEANUP);
+ if (filesShouldBeDeleted) {
+ List<Path> dirList = new ArrayList<>();
+ dirList.add(new Path(app1LogDir.toURI()));
+ verify(delSrvc, times(2)).delete(argThat(new FileDeletionMatcher(
+ delSrvc, user, null, dirList)));
+
+ String containerIdStr = container11.toString();
+ File containerLogDir = new File(app1LogDir, containerIdStr);
+ for (String fileType : new String[]{"stdout", "stderr", "syslog"}) {
+ File f = new File(containerLogDir, fileType);
+ Thread.sleep(5000);
+ Assert.assertFalse("File [" + f + "] was not deleted", f.exists());
}
- Assert.assertFalse("File [" + f + "] was not deleted", f.exists());
- }
- count = 0;
- while ((app1LogDir.exists()) && (count < maxAttempts)) {
- count++;
- Thread.sleep(100);
+ Assert.assertFalse("Directory [" + app1LogDir + "] was not deleted",
+ app1LogDir.exists());
+ } else {
+ List<Path> dirList = new ArrayList<>();
+ dirList.add(new Path(app1LogDir.toURI()));
+ verify(delSrvc, never()).delete(argThat(new FileDeletionMatcher(
+ delSrvc, user, null, dirList)));
+
+ String containerIdStr = container11.toString();
+ File containerLogDir = new File(app1LogDir, containerIdStr);
+ for (String fileType : new String[]{"stdout", "stderr", "syslog"}) {
+ File f = new File(containerLogDir, fileType);
+ Thread.sleep(5000);
Review Comment:
I don't think 5 sec wait is required for each file. Would you move the sleep
out of the for loop?
> Add uncleaning option for local app log file with log-aggregation enabled
> -------------------------------------------------------------------------
>
> Key: YARN-11241
> URL: https://issues.apache.org/jira/browse/YARN-11241
> Project: Hadoop YARN
> Issue Type: New Feature
> Components: log-aggregation
> Reporter: groot
> Assignee: groot
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Add uncleaning option for local app log file with log-aggregation enabled
> This will be helpful for debugging purpose.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
