[ https://issues.apache.org/jira/browse/YARN-5525?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15536720#comment-15536720 ]
Botong Huang commented on YARN-5525: ------------------------------------ Thanks [~subru] for the comment. I see your concern. Yes, I agree that both approaches (modify/extend the current {{LogAggregationService}} and make {{AppLogAggregator}} pluggable, or make the entire service pluggable) would work for me. Let me provide more details to justify why I prefer the latter one. The current {{LogAggregationService}} assumes and prepares the log directory in the form {{remoteRootDir/<user>/suffix/<appId>/<nodeId>}}. The root dir {{remoteRootDir}} is created using account NMOwner, and the subdirs starting from <user> are created under user account with loaded credentials. Now what we want to do is allow a different {{remoteRootDir}} per app, and the entire directory will be in user store and needs to be created with user account and credentials. Moreover, we also need to customize the subdir structures in it, potentially different structures for different types of apps. Currently the subdir structure is hardcoded in {{LogAggregationService}} and {{LogAggregationUtils}}. It would have been much easier with the former approach if these things (setting up the directorys) are originally implemented in {{AppLogAggregatorImpl}} so that we can simply plug in a different {{AppLogAggregator}} implememtation. Now given that they are not, it would involve a lot more work in {{LogAggregationService}} and {{LogAggregationUtils}} to achieve our goal by extending the current implementation with extra configs and at the same time make it backward compatible (falling back to the current behavior if customization configs are not present). These changes would also seem ad-hoc and might not be useful in general. By following the latter approach of making the entire service pluggable, the only required change is the new service class config and in {{ContainerManagerImpl}} where the service class is loaded. All the other {{private}} to {{protected}} changes are not essential, they are added only to make it easier for alternative implementations to inherit from the existing one. If we decided to adopt this approach, we might also need to create a interface which define the contract of the log aggregation service on top of the current patch (v3). > Make log aggregation service class configurable > ----------------------------------------------- > > Key: YARN-5525 > URL: https://issues.apache.org/jira/browse/YARN-5525 > Project: Hadoop YARN > Issue Type: Improvement > Components: log-aggregation > Reporter: Giovanni Matteo Fumarola > Assignee: Botong Huang > Priority: Minor > Attachments: YARN-5525.v1.patch, YARN-5525.v2.patch, > YARN-5525.v3.patch > > > Make the log aggregation class configurable and extensible, so that > alternative log aggregation behaviors like app specific log aggregation > directory, log aggregation format can be implemented and plugged in. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org