[ 
https://issues.apache.org/jira/browse/YARN-8126?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16429366#comment-16429366
 ] 

Rohith Sharma K S commented on YARN-8126:
-----------------------------------------

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

1. SystemServiceManagerImpl.java
{code:java}
          if (!services.add(service)) {
            int count = ignoredUserServices.containsKey(userName) ?
                ignoredUserServices.get(userName) : 0;
            ignoredUserServices.put(userName, count + 1);
            LOG.warn(
                "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);
{code}
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 
TestSystemServiceManagerImpl.java?

3. TestSystemServiceImpl
{code:java}
        Assert.assertTrue(
            "Service name doesn't exist in expected " + "userService "
                + serviceNames, serviceNames.contains(next.getName()));
{code}
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” 
anywhere?

5. Under 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources/
 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 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources//users/sync/user1/?

> [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
(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