Gour Saha commented on YARN-8048:

[~rohithsharma] thank you for the patch. Sorry for the late review. This is a 
very interesting and helpful feature and the patch looks great. Few minor 
comments which can be taken care of in an addendum patch of maybe another 
sub-task under the parent (if they make sense to you) -
1. SystemServiceManagerImpl.java
          if (!services.add(service)) {
            int count = ignoredUserServices.containsKey(userName) ?
                ignoredUserServices.get(userName) : 0;
            ignoredUserServices.put(userName, count + 1);
                "Ignoring service {} for the user {} as it is already present,"
                    + " filename = {}", service.getName(), userName, filename);
          LOG.info("Added service {} for the user {}, filename = {}",
              service.getName(), userName, filename);
I think the info log will get printed here every time the warn log is also 
printed inside the if block. Should the info log go in the else block?

2. Should we rename TestSystemServiceImpl.java to 

3. TestSystemServiceImpl
            "Service name doesn't exist in expected " + "userService "
                + serviceNames, serviceNames.contains(next.getName()));
Should we combine these 2 strings into 1 -> "Service name doesn't exist in 
expected " + "userService "

4. Do we need to add documentation about "yarn.service.system-service.dir” 

5. Under 
 should we rename the dir “users" to "system-services" to better reflect what 
these files/tests are for (and change resourcePath in 
TestSystemServiceImpl.java accordingly)

6. For an additional test to see if a dir is skipped, do you want to add a 
directory named “bad" (probably needs a dummy file under it otherwise github 
will not allow you to commit an empty dir) under the path 

> Support auto-spawning of admin configured services during bootstrap of 
> rm/apiserver
> -----------------------------------------------------------------------------------
>                 Key: YARN-8048
>                 URL: https://issues.apache.org/jira/browse/YARN-8048
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Rohith Sharma K S
>            Assignee: Rohith Sharma K S
>            Priority: Major
>             Fix For: 3.2.0
>         Attachments: YARN-8048.001.patch, YARN-8048.002.patch, 
> YARN-8048.003.patch, YARN-8048.004.patch, YARN-8048.005.patch, 
> YARN-8048.006.patch
> Goal is to support auto-spawning of admin configured services during 
> bootstrap of resourcemanager/apiserver. 
> *Requirement:* Some of the  services might required to be consumed by yarn 
> itself ex: Hbase for atsv2. Instead of depending on user installed HBase or 
> sometimes user may not required to install HBase at all, in such conditions 
> running HBase app on YARN will help for ATSv2.
> Before YARN cluster is started, admin configure these services spec and place 
> it in common location in HDFS. At the time of RM/apiServer bootstrap, these 
> services will be submitted.

This message was sent by Atlassian JIRA

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