[ https://issues.apache.org/jira/browse/YARN-8418?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16551057#comment-16551057 ]
Wangda Tan commented on YARN-8418: ---------------------------------- For logics of the patch, a couple of comments: 1) Is it possible to send the "new token arrived" message to LogAggregationService instead of handling inside ContainersManagerImpl? 2) In addition to above suggestion, From ContainerManagerImpl, it should not tell LogAggregationService that "app log aggregation enabled". Instead, it should notify LogAggregationService that new token arrived and let LogAggregationService make the decision. 3) Following logic: {code:java} 375 AppLogAggregator aggregator = appLogAggregators.get(appId); 376 if (aggregator != null) {{code} When this could be null? 4) Following logic: {code:java} 259 if (e.getCause() instanceof SecretManager.InvalidToken) { 260 aggregationDisabledApps.add(appId); 261 }{code} Then we should call it "invalidTokenApps" or some more appropriate name. We want to distinguish between this scenario and other invalid apps. 5) What happens if token arrives after AppLogAggregator removed from context? Is it possible? If yes, are we going to remove log dir for this case? 6) Have u done tests in real cluster to prove it work? Just to make sure we're pushing the right fix to 3.1.1 given we don't have much time before RC. [~sunil.gov...@gmail.com], [~suma.shivaprasad], could u help to review the patch and related logics since I will be off after tomorrow? > App local logs could leaked if log aggregation fails to initialize for the app > ------------------------------------------------------------------------------ > > Key: YARN-8418 > URL: https://issues.apache.org/jira/browse/YARN-8418 > Project: Hadoop YARN > Issue Type: Bug > Affects Versions: 2.8.0, 3.0.0-alpha1 > Reporter: Bibin A Chundatt > Assignee: Bibin A Chundatt > Priority: Critical > Attachments: YARN-8418.001.patch, YARN-8418.002.patch, > YARN-8418.003.patch, YARN-8418.004.patch > > > If log aggregation fails init createApp directory container logs could get > leaked in NM directory > For log running application restart of NM after token renewal this case is > possible/ Application submission with invalid delegation token -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org