[ 
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

Reply via email to