Rohith Sharma K S commented on YARN-8126:

Copy pasting [~gsaha] comments from YARN-8048.

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 

> [Follow up] Support auto-spawning of admin configured services during 
> bootstrap of rm
> -------------------------------------------------------------------------------------
>                 Key: YARN-8126
>                 URL: https://issues.apache.org/jira/browse/YARN-8126
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Rohith Sharma K S
>            Priority: Major
> YARN-8048 adds support auto-spawning of admin configured services during 
> bootstrap of rm. 
> This JIRA is to follow up some of the comments discussed in YARN-8048. 

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